Re: [libvirt] [PATCHv10 4/6] libvirt/qemu support persistent device modification

At 04/19/2011 03:46 PM, KAMEZAWA Hiroyuki Write: > This patch adds functions for modify domain's persistent definition. > To do error recovery in easy way, we use a copy of vmdef and update it. > > The whole sequence will be: > > make a copy of domain definition. > > if (flags & MODIFY_CONFIG) > update copied domain definition > if (flags & MODIF_LIVE) > do hotplug. > if (no error) > save copied one to the file and update cached definition. > else > discard copied definition. > > This patch is mixuture of Eric Blake's work and mine. > From: Eric Blake <eblake(a)redhat.com&gt; > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; > > (virDomainObjCopyPersistentDef): make a copy of persistent vm definition > (qemudDomainModifyDeviceFlags): add support for MODIFY_CONFIG and MODIFY_CURRENT > (qemudDomainAttach/Detach/UpdateDeviceConfig) : callbacks. now empty > --- > src/conf/domain_conf.c | 18 ++++++ > src/conf/domain_conf.h | 3 + > src/libvirt_private.syms | 1 + > src/qemu/qemu_driver.c | 148 ++++++++++++++++++++++++++++++++++++---------- > 4 files changed, 139 insertions(+), 31 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 6b733d4..bb8f0a4 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -9510,3 +9510,21 @@ cleanup: > > return ret; > } > + > + > +virDomainDefPtr > +virDomainObjCopyPersistentDef(virCapsPtr caps, virDomainObjPtr dom) > +{ > + char *xml; > + virDomainDefPtr cur, ret; > + > + cur = virDomainObjGetPersistentDef(caps, dom); > + > + xml = virDomainDefFormat(cur, VIR_DOMAIN_XML_WRITE_FLAGS); > + if (!xml) > + return NULL; > + > + ret = virDomainDefParseString(caps, xml, VIR_DOMAIN_XML_READ_FLAGS); > + > + return ret; > +} > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 6ea30b9..ddf111a 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -1288,6 +1288,9 @@ int virDomainObjSetDefTransient(virCapsPtr caps, > virDomainDefPtr > virDomainObjGetPersistentDef(virCapsPtr caps, > virDomainObjPtr domain); > +virDomainDefPtr > +virDomainObjCopyPersistentDef(virCapsPtr caps, virDomainObjPtr dom); > + > void virDomainRemoveInactive(virDomainObjListPtr doms, > virDomainObjPtr dom); > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index ba7739d..f732431 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -287,6 +287,7 @@ virDomainMemballoonModelTypeToString; > virDomainNetDefFree; > virDomainNetTypeToString; > virDomainObjAssignDef; > +virDomainObjCopyPersistentDef; > virDomainObjSetDefTransient; > virDomainObjGetPersistentDef; > virDomainObjIsDuplicate; > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 2bdf42e..4ac8f7e 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -4059,6 +4059,48 @@ static int qemudDomainUpdateDeviceLive(virDomainObjPtr vm, > return ret; > } > > +static int > +qemudDomainAttachDeviceConfig(virDomainDefPtr vmdef ATTRIBUTE_UNUSED, > + virDomainDeviceDefPtr dev) > +{ > + switch (dev->type) { > + default: > + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("persistent update of device is not supported")); > + return -1; > + } > + return 0; > +} > + > + > +static int > +qemudDomainDetachDeviceConfig(virDomainDefPtr vmdef ATTRIBUTE_UNUSED, > + virDomainDeviceDefPtr dev) > +{ > + switch (dev->type) { > + default: > + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("persistent update of device is not supported")); > + return -1; > + } > + return 0; > +} > + > +static int > +qemudDomainUpdateDeviceConfig(virDomainDefPtr vmdef ATTRIBUTE_UNUSED, > + virDomainDeviceDefPtr dev, > + bool force ATTRIBUTE_UNUSED) > +{ > + switch (dev->type) { > + default: > + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("persistent update of device is not supported")); > + return -1; > + } > + return 0; > + > +} > + > enum { > QEMUD_DEVICE_ATTACH, QEMUD_DEVICE_DETACH, QEMUD_DEVICE_UPDATE, > }; > @@ -4069,6 +4111,7 @@ static int qemudDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, > struct qemud_driver *driver = dom->conn->privateData; > virBitmapPtr qemuCaps = NULL; > virDomainObjPtr vm = NULL; > + virDomainDefPtr vmdef = NULL; > virDomainDeviceDefPtr dev = NULL; > bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; > int ret = -1; > @@ -4077,7 +4120,8 @@ static int qemudDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, > case QEMUD_DEVICE_ATTACH: > case QEMUD_DEVICE_DETACH: > virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | > - VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); > + VIR_DOMAIN_DEVICE_MODIFY_CONFIG | > + VIR_DOMAIN_DEVICE_MODIFY_CURRENT, -1); > break; > case QEMUD_DEVICE_UPDATE: > virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_CURRENT | > @@ -4089,12 +4133,6 @@ static int qemudDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, > break; > } > > - if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { > - qemuReportError(VIR_ERR_OPERATION_INVALID, > - "%s", _("cannot modify the persistent configuration of a domain")); > - return -1; > - } > - > qemuDriverLock(driver); > vm = virDomainFindByUUID(&driver->domains, dom->uuid); > if (!vm) { > @@ -4108,11 +4146,29 @@ static int qemudDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, > if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) > goto cleanup; > > - if (!virDomainObjIsActive(vm)) { > - qemuReportError(VIR_ERR_OPERATION_INVALID, > - "%s", _("cannot attach device on inactive domain")); > - goto endjob; > + if (virDomainObjIsActive(vm)) { > + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CURRENT) > + flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE; VIR_DOMAIN_DEVICE_MODIFY_CURRENT is 0. So you should check 'flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT' instead of 'flags & VIR_DOMAIN_DEVICE_MODIFY_CURRENT'.

> + } else { > + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CURRENT) > + flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG; Tha same as above.