Re: [libvirt] [PATCH v2 10/14] libxl: support domain config modification in virDomainRestoreFlags

Marek Marczykowski-Górecki wrote: > Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com&gt; > --- > src/libxl/libxl_driver.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index 8dec70b..7b50853 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -2306,11 +2306,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, > int ret = -1; > > virCheckFlags(VIR_DOMAIN_SAVE_PAUSED, -1); > - if (dxml) { > - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", > - _("xml modification unsupported")); > - return -1; > - } > > libxlDriverLock(driver); > > @@ -2318,6 +2313,19 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, > if (fd < 0) > goto cleanup; > > + if (dxml) { > What kind of changes are supported in dxml? I tried passing xml that contained another vif, which caused libxl to fail the restore

libxl: error: libxl_device.c:878:device_backend_callback: unable to add device with path /local/domain/0/backend/vif/25/0 > + virDomainDefPtr def2 = NULL; > + > + if (!(def2 = virDomainDefParseString(dxml, driver->caps, driver->xmlopt, > + 1 << VIR_DOMAIN_VIRT_XEN, > + VIR_DOMAIN_XML_INACTIVE))) { > + goto cleanup; > + } > + virDomainDefFree(def); > + def = def2; > But we've already thrown away the original config and replaced it with config that subsequently fails. The qemu driver calls virDomainDefCheckABIStability to ensure the new config is compatible, and IMO we should do the same here.

Regards, Jim > + } > + > + > if (!(vm = virDomainObjListAdd(driver->domains, def, > driver->xmlopt, > VIR_DOMAIN_OBJ_LIST_ADD_LIVE | >