[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 redhat com>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa hiroyu jp fujitsu com>
> 
> (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);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]