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'.
> + } else {
> + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CURRENT)
> + flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
Tha same as above.
> + /* check consistency between flags and the vm state */
> + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> + "%s",
> + _("cannot modify device on inactive domain"));
> + goto endjob;
> + }
> + }
> +
> + if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> + "%s", _("cannot modify device on transient domain"));
> + goto endjob;
> }
> + /* At updating config, we update a copy */
> + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)
> + vmdef = virDomainObjCopyPersistentDef(driver->caps, vm);
>
> dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
> VIR_DOMAIN_XML_INACTIVE);
> @@ -4124,33 +4180,63 @@ static int qemudDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
> &qemuCaps) < 0)
> goto endjob;
>
> - switch (action) {
> - case QEMUD_DEVICE_ATTACH:
> - ret = qemudDomainAttachDeviceLive(vm, dev, dom, qemuCaps);
> - break;
> - case QEMUD_DEVICE_DETACH:
> - ret = qemudDomainDetachDeviceLive(vm, dev, dom, qemuCaps);
> - break;
> - case QEMUD_DEVICE_UPDATE:
> - ret = qemudDomainUpdateDeviceLive(vm, dev, dom, qemuCaps, force);
> - break;
> - default:
> - break;
> - }
> + ret = 0;
>
> - /*
> - * update domain status forcibly because the domain status may be changed
> - * even if we attach the device failed. For example, a new controller may
> - * be created.
> - */
> - if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
> - ret = -1;
> + /* Update a copy of persistent definition */
> + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
> + switch (action) {
> + case QEMUD_DEVICE_ATTACH:
> + ret = qemudDomainAttachDeviceConfig(vmdef, dev);
> + break;
> + case QEMUD_DEVICE_DETACH:
> + ret = qemudDomainDetachDeviceConfig(vmdef, dev);
> + break;
> + case QEMUD_DEVICE_UPDATE:
> + ret = qemudDomainUpdateDeviceConfig(vmdef, dev, force);
> + break;
> + default:
> + break;
> + }
> + }
> + /* Update Live */
> + if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)) {
> + switch (action) {
> + case QEMUD_DEVICE_ATTACH:
> + ret = qemudDomainAttachDeviceLive(vm, dev, dom, qemuCaps);
> + break;
> + case QEMUD_DEVICE_DETACH:
> + ret = qemudDomainDetachDeviceLive(vm, dev, dom, qemuCaps);
> + break;
> + case QEMUD_DEVICE_UPDATE:
> + ret = qemudDomainUpdateDeviceLive(vm, dev, dom, qemuCaps, force);
> + break;
> + default:
> + break;
> + }
> + /*
> + * update domain status forcibly because the domain status may be
> + * changed even if we attach the device failed. For example, a new
> + * controller may be created.
> + */
> + if (!ret &&
> + virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
> + ret = -1;
> + }
> + /* No error until here, we can save persistent definition */
> + if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) {
> + ret = virDomainSaveConfig(driver->configDir, vmdef);
> + if (!ret) {
> + virDomainObjAssignDef(vm, vmdef, false);
> + vmdef = NULL;
> + }
> + }
>
> endjob:
> if (qemuDomainObjEndJob(vm) == 0)
> vm = NULL;
>
> cleanup:
> + virDomainDefFree(vmdef);
> virDomainDeviceDefFree(dev);
> if (vm)
> virDomainObjUnlock(vm);
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list