Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
On Wed, 2015-05-06 at 11:59 -0600, Jim Fehlig wrote: > Dario Faggioli wrote: > > On Fri, 2015-05-01 at 15:32 -0600, Jim Fehlig wrote: > >> E.g. if a domain is configured to use memory allocated from certain numa > >> nodes, we'd want to make sure the dest has sufficient memory amongst > >> those nodes. > >> > >> But these were just examples of things we may want to check before > >> starting the migration. I suppose there are better examples wrt > >> storage, e.g. does the domain have an attached DVD, ... > >> > >> > > > > Do (if you know that) other drivers in libvirt do these kind of checks? > > > > Yes. The qemu driver has quite extensive migration checks. > Interesting. I never had the chance to check that. I think we should have a look and try to figure out whether some o them make sense for us too, and decide whether we want them in libxl or in libvirt. I'm adding this to my TODO list. Thanks and Regards, Dario signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
Dario Faggioli wrote: > On Fri, 2015-05-01 at 15:32 -0600, Jim Fehlig wrote: > >> Dario Faggioli wrote: >> > > >>> I mean what NUMA configuration are we talking about, and when and under >>> what circumstances would you consider it "migratable"? >>> >>> >> E.g. if a domain is configured to use memory allocated from certain numa >> nodes, we'd want to make sure the dest has sufficient memory amongst >> those nodes. >> >> > Right. I was asking because I'm interested in the problem at the libxl > level. To the best of my knowledge, I don't think we do anything like > that in libxl for now, and I still am not sure whether we should... :-) > > >> But these were just examples of things we may want to check before >> starting the migration. I suppose there are better examples wrt >> storage, e.g. does the domain have an attached DVD, ... >> >> > Exactly. > > Do (if you know that) other drivers in libvirt do these kind of checks? > Yes. The qemu driver has quite extensive migration checks. Regards, Jim ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
On Fri, 2015-05-01 at 15:32 -0600, Jim Fehlig wrote: > Dario Faggioli wrote: > > I mean what NUMA configuration are we talking about, and when and under > > what circumstances would you consider it "migratable"? > > > > E.g. if a domain is configured to use memory allocated from certain numa > nodes, we'd want to make sure the dest has sufficient memory amongst > those nodes. > Right. I was asking because I'm interested in the problem at the libxl level. To the best of my knowledge, I don't think we do anything like that in libxl for now, and I still am not sure whether we should... :-) > But these were just examples of things we may want to check before > starting the migration. I suppose there are better examples wrt > storage, e.g. does the domain have an attached DVD, ... > Exactly. Do (if you know that) other drivers in libvirt do these kind of checks? Regards, Dario signature.asc Description: This is a digitally signed message part ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
Dario Faggioli wrote:
> On Tue, 2015-04-14 at 20:15 -0600, Jim Fehlig wrote:
>
>> Konrad Rzeszutek Wilk wrote:
>>
>
>
>>> -static bool
>>> -libxlDomainMigrationIsAllowed(virDomainDefPtr def)
>>> -{
>>> -/* Migration is not allowed if definition contains any hostdevs */
>>> -if (def->nhostdevs > 0) {
>>> -virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>>> - _("domain has assigned host devices"));
>>> -return false;
>>> -}
>>> -
>>> -return true;
>>> -}
>>> -
>>>
>> This function was created with the intention of adding more checks, e.g.
>> is the cpu and numa configuration migratable?
>>
>>
> Only vaguely related to the patch, I know (sorry!), but if I can ask,
> what do you mean with "is the cpu and numa configuration migratable?"
>
I was thinking about domXML such as
...
8
62914560
104857600
...
> I mean what NUMA configuration are we talking about, and when and under
> what circumstances would you consider it "migratable"?
>
E.g. if a domain is configured to use memory allocated from certain numa
nodes, we'd want to make sure the dest has sufficient memory amongst
those nodes.
But these were just examples of things we may want to check before
starting the migration. I suppose there are better examples wrt
storage, e.g. does the domain have an attached DVD, ...
Regards,
Jim
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
On Tue, 2015-04-14 at 20:15 -0600, Jim Fehlig wrote:
> Konrad Rzeszutek Wilk wrote:
> > -static bool
> > -libxlDomainMigrationIsAllowed(virDomainDefPtr def)
> > -{
> > -/* Migration is not allowed if definition contains any hostdevs */
> > -if (def->nhostdevs > 0) {
> > -virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> > - _("domain has assigned host devices"));
> > -return false;
> > -}
> > -
> > -return true;
> > -}
> > -
>
> This function was created with the intention of adding more checks, e.g.
> is the cpu and numa configuration migratable?
>
Only vaguely related to the patch, I know (sorry!), but if I can ask,
what do you mean with "is the cpu and numa configuration migratable?"
I mean what NUMA configuration are we talking about, and when and under
what circumstances would you consider it "migratable"?
Thanks and Regards,
Dario
signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
On Tue, Apr 14, 2015 at 08:15:52PM -0600, Jim Fehlig wrote:
> Konrad Rzeszutek Wilk wrote:
> > It is unhealthy. If the device is not doing any DMA operations
> > it would work - but if you are saving and there are DMA operations
> > happening the chance of corruption (outstanding DMAs) increase.
> >
> > As such re-use the check migration used.
> >
>
> s/used// ?
>
> > Signed-off-by: Konrad Rzeszutek Wilk
> > ---
> > src/libxl/libxl_domain.c| 19 +++
> > src/libxl/libxl_domain.h| 3 +++
> > src/libxl/libxl_driver.c| 6 ++
> > src/libxl/libxl_migration.c | 15 +--
> > 4 files changed, 29 insertions(+), 14 deletions(-)
> >
> > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > index 5e0ab56..c038989 100644
> > --- a/src/libxl/libxl_domain.c
> > +++ b/src/libxl/libxl_domain.c
> > @@ -178,6 +178,25 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver
> > ATTRIBUTE_UNUSED,
> > return virObjectUnref(obj);
> > }
> >
> > +/*
> > + * Checks whether the domain has host devices (PCIe) to disallow
> > + * migration or saving of guest - unless they are detached.
> > + *
> > + * Returns true if the domain has host devices, otherwise false.
> > + */
> > +bool
> > +libxlDomainHasHostDevices(virDomainDefPtr def)
> > +{
> > +/* Migration is not allowed if definition contains any hostdevs */
> > +if (def->nhostdevs > 0) {
> > +virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> > + _("domain has assigned host devices"));
> > +return true;
> > +}
> > +
> > +return false;
> > +}
> > +
> > static void *
> > libxlDomainObjPrivateAlloc(void)
> > {
> > diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
> > index aa647b8..76c0c8d 100644
> > --- a/src/libxl/libxl_domain.h
> > +++ b/src/libxl/libxl_domain.h
> > @@ -95,6 +95,9 @@ char *
> > libxlDomainManagedSavePath(libxlDriverPrivatePtr driver,
> > virDomainObjPtr vm);
> >
> > +bool
> > +libxlDomainHasHostDevices(virDomainDefPtr def);
> > +
> > int
> > libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver,
> > libxlDriverConfigPtr cfg,
> > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> > index 9eb071e..b9faba8 100644
> > --- a/src/libxl/libxl_driver.c
> > +++ b/src/libxl/libxl_driver.c
> > @@ -1650,6 +1650,9 @@ libxlDomainSaveFlags(virDomainPtr dom, const char
> > *to, const char *dxml,
> > goto endjob;
> > }
> >
> > +if (libxlDomainHasHostDevices(vm->def))
> > +goto endjob;
> > +
> > if (libxlDoDomainSave(driver, vm, to) < 0)
> > goto endjob;
> >
> > @@ -1876,6 +1879,9 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int
> > flags)
> > goto endjob;
> > }
> >
> > +if (libxlDomainHasHostDevices(vm->def))
> > +goto endjob;
> > +
> > name = libxlDomainManagedSavePath(driver, vm);
> > if (name == NULL)
> > goto endjob;
> > diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
> > index 4010506..aaed448c 100644
> > --- a/src/libxl/libxl_migration.c
> > +++ b/src/libxl/libxl_migration.c
> > @@ -209,19 +209,6 @@ libxlDoMigrateSend(libxlDriverPrivatePtr driver,
> > return ret;
> > }
> >
> > -static bool
> > -libxlDomainMigrationIsAllowed(virDomainDefPtr def)
> > -{
> > -/* Migration is not allowed if definition contains any hostdevs */
> > -if (def->nhostdevs > 0) {
> > -virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> > - _("domain has assigned host devices"));
> > -return false;
> > -}
> > -
> > -return true;
> > -}
> > -
>
> This function was created with the intention of adding more checks, e.g.
> is the cpu and numa configuration migratable? Are there pending
> snapshot or block jobs (once those are supported)? ...
>
> I'd like to keep libxlDomainMigrationIsAllowed even though it currently
> only checks for assigned hostdevs. I think it improves readability of
> the migration code. For the time it could simply call
> libxlDomainHasHostDevices.
You got it.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
Konrad Rzeszutek Wilk wrote:
> It is unhealthy. If the device is not doing any DMA operations
> it would work - but if you are saving and there are DMA operations
> happening the chance of corruption (outstanding DMAs) increase.
>
> As such re-use the check migration used.
>
s/used// ?
> Signed-off-by: Konrad Rzeszutek Wilk
> ---
> src/libxl/libxl_domain.c| 19 +++
> src/libxl/libxl_domain.h| 3 +++
> src/libxl/libxl_driver.c| 6 ++
> src/libxl/libxl_migration.c | 15 +--
> 4 files changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 5e0ab56..c038989 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -178,6 +178,25 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver
> ATTRIBUTE_UNUSED,
> return virObjectUnref(obj);
> }
>
> +/*
> + * Checks whether the domain has host devices (PCIe) to disallow
> + * migration or saving of guest - unless they are detached.
> + *
> + * Returns true if the domain has host devices, otherwise false.
> + */
> +bool
> +libxlDomainHasHostDevices(virDomainDefPtr def)
> +{
> +/* Migration is not allowed if definition contains any hostdevs */
> +if (def->nhostdevs > 0) {
> +virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("domain has assigned host devices"));
> +return true;
> +}
> +
> +return false;
> +}
> +
> static void *
> libxlDomainObjPrivateAlloc(void)
> {
> diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
> index aa647b8..76c0c8d 100644
> --- a/src/libxl/libxl_domain.h
> +++ b/src/libxl/libxl_domain.h
> @@ -95,6 +95,9 @@ char *
> libxlDomainManagedSavePath(libxlDriverPrivatePtr driver,
> virDomainObjPtr vm);
>
> +bool
> +libxlDomainHasHostDevices(virDomainDefPtr def);
> +
> int
> libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver,
> libxlDriverConfigPtr cfg,
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 9eb071e..b9faba8 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -1650,6 +1650,9 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to,
> const char *dxml,
> goto endjob;
> }
>
> +if (libxlDomainHasHostDevices(vm->def))
> +goto endjob;
> +
> if (libxlDoDomainSave(driver, vm, to) < 0)
> goto endjob;
>
> @@ -1876,6 +1879,9 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int
> flags)
> goto endjob;
> }
>
> +if (libxlDomainHasHostDevices(vm->def))
> +goto endjob;
> +
> name = libxlDomainManagedSavePath(driver, vm);
> if (name == NULL)
> goto endjob;
> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
> index 4010506..aaed448c 100644
> --- a/src/libxl/libxl_migration.c
> +++ b/src/libxl/libxl_migration.c
> @@ -209,19 +209,6 @@ libxlDoMigrateSend(libxlDriverPrivatePtr driver,
> return ret;
> }
>
> -static bool
> -libxlDomainMigrationIsAllowed(virDomainDefPtr def)
> -{
> -/* Migration is not allowed if definition contains any hostdevs */
> -if (def->nhostdevs > 0) {
> -virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("domain has assigned host devices"));
> -return false;
> -}
> -
> -return true;
> -}
> -
This function was created with the intention of adding more checks, e.g.
is the cpu and numa configuration migratable? Are there pending
snapshot or block jobs (once those are supported)? ...
I'd like to keep libxlDomainMigrationIsAllowed even though it currently
only checks for assigned hostdevs. I think it improves readability of
the migration code. For the time it could simply call
libxlDomainHasHostDevices.
Regards,
Jim
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
