[libvirt] [PATCH] libxl: Disallow save or migrate when host devices are assigned to a guest.
Konrad Rzeszutek Wilk
konrad.wilk at oracle.com
Wed Apr 15 13:08:16 UTC 2015
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// ?
<nods>
>
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
> > ---
> > 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.
More information about the libvir-list
mailing list