On Wed, 20 Apr 2011 10:52:41 +0800
Wen Congyang <wency@xxxxxxxxxxxxxx> wrote:
> 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@xxxxxxxxxx>
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> >
> > (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'.
>
Hmm, ok. I missed that.
> > + } else {
> > + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
> > + flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
>
> Tha same as above.
>
Sure.
Thanks,
-Kame
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list