[libvirt-users] [PATCHv11 0/6] libvirt/qemu - persistent modification of devices
Here is v11. Fixed comments/bugs and updated against the latest libvirt.git. Changes v10->v11: - fixed comments on each patches - fixed cgroup handling in patch 3. - fixed MODIFY_CURRENT handling in patch 4. most of diff comes from refactoring qemu/qemu_driver.c -- conf/domain_conf.c | 40 ++ conf/domain_conf.h | 5 libvirt_private.syms | 3 qemu/qemu_driver.c | 727 +++++++++++++++++++++++++++++++-------------------- 4 files changed, 500 insertions(+), 275 deletions(-) Thanks, -Kame
Centralize device modification in the more flexible APIs, to allow future honoring of additional flags. Explicitly reject the VIR_DOMAIN_DEVICE_MODIFY_FORCE flag on attach/detach. Based on Eric Blake<eblake@redhat.com>'s work. From: Eric Blake <eblake@redhat.com> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> * src/qemu/qemu_driver.c (qemudDomainAttachDevice)(qemudDomainAttachDeviceFlags): Swap bodies,rename... (qemudDomainDetachDevice, qemudDomainDetachDeviceFlags): Likewise. --- src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++++--------------------- 1 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f6e503a..a8f3849 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3786,8 +3786,8 @@ cleanup: } -static int qemudDomainAttachDevice(virDomainPtr dom, - const char *xml) +static int qemudDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -3796,6 +3796,14 @@ static int qemudDomainAttachDevice(virDomainPtr dom, virCgroupPtr cgroup = NULL; int ret = -1; + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + 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) { @@ -3943,16 +3951,10 @@ cleanup: return ret; } -static int qemudDomainAttachDeviceFlags(virDomainPtr dom, - const char *xml, - unsigned int flags) { - if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot modify the persistent configuration of a domain")); - return -1; - } - - return qemudDomainAttachDevice(dom, xml); +static int qemudDomainAttachDevice(virDomainPtr dom, const char *xml) +{ + return qemudDomainAttachDeviceFlags(dom, xml, + VIR_DOMAIN_DEVICE_MODIFY_LIVE); } @@ -4078,14 +4080,23 @@ cleanup: } -static int qemudDomainDetachDevice(virDomainPtr dom, - const char *xml) { +static int qemudDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) +{ struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; virBitmapPtr qemuCaps = NULL; virDomainDeviceDefPtr dev = NULL; int ret = -1; + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE| + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + + 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) { @@ -4164,16 +4175,10 @@ cleanup: return ret; } -static int qemudDomainDetachDeviceFlags(virDomainPtr dom, - const char *xml, - unsigned int flags) { - if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot modify the persistent configuration of a domain")); - return -1; - } - - return qemudDomainDetachDevice(dom, xml); +static int qemudDomainDetachDevice(virDomainPtr dom, const char *xml) +{ + return qemudDomainDetachDeviceFlags(dom, xml, + VIR_DOMAIN_DEVICE_MODIFY_LIVE); } static int qemudDomainGetAutostart(virDomainPtr dom, -- 1.7.4.1
clean up At(De)tachDeviceFlags() for consolidation. qemudDomainAttachDeviceFlags()/qemudDomainDetachFlags()/ qemudDomainUpdateDeviceFlags() has similar logics and copied codes. This patch series tries to unify them to use shared code when it can. At first, clean up At(De)tachDeviceFlags() and devide it into functions. By this, this patch pulls out shared components between functions. Based on patch series by Eric Blake, I added some modification as switch-case with QEMUD_DEVICE_ATTACH, QEMUD_DEVICE_DETACH, QEMUD_DEVICE_UPDATE From: Eric Blake <eblake@redhat.com> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> * src/qemu/qemu_driver.c (qemudDomainAt(De)tachDeviceFlags) : pulled out to qemudDomainModifyDeviceFlags() (qemudDomainModifyDeviceFlags) : impelements a generic codes for modify domain. (qemudDomainAt(De)tachDeviceFlagsLive) : codes for at(de)taching devices to domain in line. no changes in logic from old codes. (qemudDomainAt(De)tachDeviceDiskLive) : for at(de)taching Disks. (qemudDomainAt(De)tachDeviceControllerLive) : for at(de)taching Controllers --- src/qemu/qemu_driver.c | 429 ++++++++++++++++++++++++++---------------------- 1 files changed, 234 insertions(+), 195 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a8f3849..f33a7f4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3785,15 +3785,223 @@ cleanup: return ret; } +static int qemudDomainAttachDeviceDiskLive(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virBitmapPtr qemuCaps) +{ + virDomainDiskDefPtr disk = dev->data.disk; + virCgroupPtr cgroup = NULL; + int ret = -1; -static int qemudDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, - unsigned int flags) + if (disk->driverName != NULL && !STREQ(disk->driverName, "qemu")) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported driver name '%s' for disk '%s'"), + disk->driverName, disk->src); + goto end; + } + + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find cgroup for %s"), + vm->def->name); + goto end; + } + if (qemuSetupDiskCgroup(driver, vm, cgroup, disk) < 0) + goto end; + } + switch (disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: + ret = qemuDomainChangeEjectableMedia(driver, vm, disk, qemuCaps, false); + break; + case VIR_DOMAIN_DISK_DEVICE_DISK: + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) + ret = qemuDomainAttachUsbMassstorageDevice(driver, vm, + disk, qemuCaps); + else if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) + ret = qemuDomainAttachPciDiskDevice(driver, vm, disk, qemuCaps); + else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) + ret = qemuDomainAttachSCSIDisk(driver, vm, disk, qemuCaps); + else + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be hotplugged."), + virDomainDiskBusTypeToString(disk->bus)); + break; + default: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk device type '%s' cannot be hotplugged"), + virDomainDiskDeviceTypeToString(disk->device)); + break; + } + + if (ret != 0 && cgroup) { + if (qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0) + VIR_WARN("Failed to teardown cgroup for disk path %s", + NULLSTR(disk->src)); + } +end: + if (cgroup) + virCgroupFree(&cgroup); + return ret; +} + +static int +qemudDomainAttachDeviceControllerLive(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virBitmapPtr qemuCaps) +{ + virDomainControllerDefPtr cont = dev->data.controller; + int ret = -1; + + switch (cont->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: + ret = qemuDomainAttachPciControllerDevice(driver, vm, cont, qemuCaps); + break; + default: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk controller bus '%s' cannot be hotplugged."), + virDomainControllerTypeToString(cont->type)); + break; + } + return ret; +} + +static int qemudDomainAttachDeviceLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom, + virBitmapPtr qemuCaps) +{ + struct qemud_driver *driver = dom->conn->privateData; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + ret = qemudDomainAttachDeviceDiskLive(driver, vm, dev, qemuCaps); + if (!ret) + dev->data.disk = NULL; + break; + + case VIR_DOMAIN_DEVICE_CONTROLLER: + ret = qemudDomainAttachDeviceControllerLive(driver, vm, dev, qemuCaps); + if (!ret) + dev->data.controller = NULL; + break; + + case VIR_DOMAIN_DEVICE_NET: + ret = qemuDomainAttachNetDevice(dom->conn, driver, vm, + dev->data.net, qemuCaps); + if (!ret) + dev->data.net = NULL; + break; + + case VIR_DOMAIN_DEVICE_HOSTDEV: + ret = qemuDomainAttachHostDevice(driver, vm, + dev->data.hostdev, qemuCaps); + if (!ret) + dev->data.hostdev = NULL; + break; + + default: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot be attached"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} + +static int qemudDomainDetachDeviceDiskLive(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virBitmapPtr qemuCaps) +{ + virDomainDiskDefPtr disk = dev->data.disk; + int ret = -1; + + switch (disk->device) { + case VIR_DOMAIN_DISK_DEVICE_DISK: + if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) + ret = qemuDomainDetachPciDiskDevice(driver, vm, dev, qemuCaps); + else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) + ret = qemuDomainDetachDiskDevice(driver, vm, dev, qemuCaps); + else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) + ret = qemuDomainDetachDiskDevice(driver, vm, dev, qemuCaps); + else + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This type of disk cannot be hot unplugged")); + break; + default: + break; + } + return ret; +} + +static int +qemudDomainDetachDeviceControllerLive(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virBitmapPtr qemuCaps) +{ + virDomainControllerDefPtr cont = dev->data.controller; + int ret = -1; + + switch (cont->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: + ret = qemuDomainDetachPciControllerDevice(driver, vm, dev, qemuCaps); + break; + default : + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk controller bus '%s' cannot be hotunplugged."), + virDomainControllerTypeToString(cont->type)); + } + return ret; +} + +static int qemudDomainDetachDeviceLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom, + virBitmapPtr qemuCaps) +{ + struct qemud_driver *driver = dom->conn->privateData; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + ret = qemudDomainDetachDeviceDiskLive(driver, vm, dev, qemuCaps); + break; + case VIR_DOMAIN_DEVICE_CONTROLLER: + ret = qemudDomainDetachDeviceControllerLive(driver, vm, dev, qemuCaps); + break; + case VIR_DOMAIN_DEVICE_NET: + ret = qemuDomainDetachNetDevice(driver, vm, dev, qemuCaps); + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: + ret = qemuDomainDetachHostDevice(driver, vm, dev, qemuCaps); + break; + default: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("This type of device cannot be hot unplugged")); + break; + } + + return ret; +} + +enum { + QEMUD_DEVICE_ATTACH, QEMUD_DEVICE_DETACH, QEMUD_DEVICE_UPDATE, +}; + +static int qemudDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags, int action) { struct qemud_driver *driver = dom->conn->privateData; - virDomainObjPtr vm; - virDomainDeviceDefPtr dev = NULL; virBitmapPtr qemuCaps = NULL; - virCgroupPtr cgroup = NULL; + virDomainObjPtr vm = NULL; + virDomainDeviceDefPtr dev = NULL; int ret = -1; virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | @@ -3833,106 +4041,22 @@ static int qemudDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, &qemuCaps) < 0) goto endjob; - if (dev->type == VIR_DOMAIN_DEVICE_DISK) { - if (dev->data.disk->driverName != NULL && - !STREQ(dev->data.disk->driverName, "qemu")) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported driver name '%s' for disk '%s'"), - dev->data.disk->driverName, dev->data.disk->src); - goto endjob; - } - - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - goto endjob; - } - if (qemuSetupDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0) - goto endjob; - } - - switch (dev->data.disk->device) { - case VIR_DOMAIN_DISK_DEVICE_CDROM: - case VIR_DOMAIN_DISK_DEVICE_FLOPPY: - ret = qemuDomainChangeEjectableMedia(driver, vm, - dev->data.disk, - qemuCaps, - false); - if (ret == 0) - dev->data.disk = NULL; - break; - - case VIR_DOMAIN_DISK_DEVICE_DISK: - if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) { - ret = qemuDomainAttachUsbMassstorageDevice(driver, vm, - dev->data.disk, qemuCaps); - if (ret == 0) - dev->data.disk = NULL; - } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { - ret = qemuDomainAttachPciDiskDevice(driver, vm, - dev->data.disk, qemuCaps); - if (ret == 0) - dev->data.disk = NULL; - } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) { - ret = qemuDomainAttachSCSIDisk(driver, vm, - dev->data.disk, qemuCaps); - if (ret == 0) - dev->data.disk = NULL; - } else { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk bus '%s' cannot be hotplugged."), - virDomainDiskBusTypeToString(dev->data.disk->bus)); - /* fallthrough */ - } - break; - - default: - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk device type '%s' cannot be hotplugged"), - virDomainDiskDeviceTypeToString(dev->data.disk->device)); - /* Fallthrough */ - } - if (ret != 0 && cgroup) { - if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0) - VIR_WARN("Failed to teardown cgroup for disk path %s", - NULLSTR(dev->data.disk->src)); - } - } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { - if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { - ret = qemuDomainAttachPciControllerDevice(driver, vm, - dev->data.controller, qemuCaps); - if (ret == 0) - dev->data.controller = NULL; - } else { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk controller bus '%s' cannot be hotplugged."), - virDomainControllerTypeToString(dev->data.controller->type)); - /* fallthrough */ - } - } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { - ret = qemuDomainAttachNetDevice(dom->conn, driver, vm, - dev->data.net, qemuCaps); - if (ret == 0) - dev->data.net = NULL; - } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { - ret = qemuDomainAttachHostDevice(driver, vm, - dev->data.hostdev, qemuCaps); - if (ret == 0) - dev->data.hostdev = NULL; - } else { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("device type '%s' cannot be attached"), - virDomainDeviceTypeToString(dev->type)); - 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; + default: + break; } - - /* update domain status forcibly because the domain status may be changed + /* + * 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 (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; endjob: @@ -3940,10 +4064,6 @@ endjob: vm = NULL; cleanup: - if (cgroup) - virCgroupFree(&cgroup); - - qemuCapsFree(qemuCaps); virDomainDeviceDefFree(dev); if (vm) virDomainObjUnlock(vm); @@ -3951,6 +4071,13 @@ cleanup: return ret; } +static int qemudDomainAttachDeviceFlags(virDomainPtr dom, + const char *xml, + unsigned int flags) +{ + return qemudDomainModifyDeviceFlags(dom, xml, flags, QEMUD_DEVICE_ATTACH); +} + static int qemudDomainAttachDevice(virDomainPtr dom, const char *xml) { return qemudDomainAttachDeviceFlags(dom, xml, @@ -4080,99 +4207,11 @@ cleanup: } + static int qemudDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { - struct qemud_driver *driver = dom->conn->privateData; - virDomainObjPtr vm; - virBitmapPtr qemuCaps = NULL; - virDomainDeviceDefPtr dev = NULL; - int ret = -1; - - virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE| - VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); - - 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) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - qemuReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; - } - - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) - goto cleanup; - - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot detach device on inactive domain")); - goto endjob; - } - - dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, - VIR_DOMAIN_XML_INACTIVE); - if (dev == NULL) - goto endjob; - - if (qemuCapsExtractVersionInfo(vm->def->emulator, vm->def->os.arch, - NULL, - &qemuCaps) < 0) - goto endjob; - - if (dev->type == VIR_DOMAIN_DEVICE_DISK && - dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { - ret = qemuDomainDetachPciDiskDevice(driver, vm, dev, qemuCaps); - } - else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) { - ret = qemuDomainDetachDiskDevice(driver, vm, dev, qemuCaps); - } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) { - ret = qemuDomainDetachDiskDevice(driver, vm, dev, qemuCaps); - } - else { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This type of disk cannot be hot unplugged")); - } - } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { - ret = qemuDomainDetachNetDevice(driver, vm, dev, qemuCaps); - } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { - if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { - ret = qemuDomainDetachPciControllerDevice(driver, vm, dev, - qemuCaps); - } else { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk controller bus '%s' cannot be hotunplugged."), - virDomainControllerTypeToString(dev->data.controller->type)); - /* fallthrough */ - } - } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { - ret = qemuDomainDetachHostDevice(driver, vm, dev, qemuCaps); - } else { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("This type of device cannot be hot unplugged")); - } - - if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) - ret = -1; - -endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; - -cleanup: - qemuCapsFree(qemuCaps); - virDomainDeviceDefFree(dev); - if (vm) - virDomainObjUnlock(vm); - qemuDriverUnlock(driver); - return ret; + return qemudDomainModifyDeviceFlags(dom, xml, flags, QEMUD_DEVICE_DETACH); } static int qemudDomainDetachDevice(virDomainPtr dom, const char *xml) -- 1.7.4.1
This patch strips reusable part of qemudDomainUpdateDeviceFlags() and consolidate it to qemudDomainModifyDeviceFlags(). No functional changes. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Changelog: v10->v11 - fixed qemuTeardownDiskCgroup() * src/qemu/qemu_driver.c (qemudDomainChangeDiskMediaLive) : pulled out code for updating disks. (qemudDomainUpdateDeviceLive) : core of UpdateDevice, extracted from UpdateDeviceFlags() (qemudDomainModifyDeviceFlags): add support for updating device in live domain. (qemudDomainUpdateDeviceFlags): reworked as a wrapper function of qemudDomainModifyDeviceFlags() --- src/qemu/qemu_driver.c | 215 +++++++++++++++++++++-------------------------- 1 files changed, 96 insertions(+), 119 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f33a7f4..4cba9d0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3991,6 +3991,80 @@ static int qemudDomainDetachDeviceLive(virDomainObjPtr vm, return ret; } +static int +qemudDomainChangeDiskMediaLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + struct qemud_driver *driver, + virBitmapPtr qemuCaps, + bool force) +{ + virDomainDiskDefPtr disk = dev->data.disk; + virCgroupPtr cgroup = NULL; + int ret; + + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { + if (virCgroupForDomain(driver->cgroup, + vm->def->name, &cgroup, 0) !=0 ) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find cgroup for %s"), + vm->def->name); + goto end; + } + if (qemuSetupDiskCgroup(driver, vm, cgroup, disk) < 0) + goto end; + } + + switch (disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: + ret = qemuDomainChangeEjectableMedia(driver, vm, disk, qemuCaps, force); + if (ret == 0) + dev->data.disk = NULL; + break; + default: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be updated."), + virDomainDiskBusTypeToString(disk->bus)); + break; + } + + if (ret != 0 && cgroup) { + if (qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0) + VIR_WARN("Failed to teardown cgroup for disk path %s", + NULLSTR(disk->src)); + } +end: + if (cgroup) + virCgroupFree(&cgroup); + return ret; +} + +static int qemudDomainUpdateDeviceLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom, + virBitmapPtr qemuCaps, + bool force) +{ + struct qemud_driver *driver = dom->conn->privateData; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + ret = qemudDomainChangeDiskMediaLive(vm, dev, driver, qemuCaps, force); + break; + case VIR_DOMAIN_DEVICE_GRAPHICS: + ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics); + break; + default: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot be updated"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} + enum { QEMUD_DEVICE_ATTACH, QEMUD_DEVICE_DETACH, QEMUD_DEVICE_UPDATE, }; @@ -4002,10 +4076,25 @@ static int qemudDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, virBitmapPtr qemuCaps = NULL; virDomainObjPtr vm = NULL; virDomainDeviceDefPtr dev = NULL; + bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; int ret = -1; - virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | - VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + switch (action) { + case QEMUD_DEVICE_ATTACH: + case QEMUD_DEVICE_DETACH: + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + break; + case QEMUD_DEVICE_UPDATE: + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_CURRENT | + VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG | + VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1); + break; + default: + break; + } + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot modify the persistent configuration of a domain")); @@ -4048,9 +4137,13 @@ static int qemudDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, 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 @@ -4089,125 +4182,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { - struct qemud_driver *driver = dom->conn->privateData; - virDomainObjPtr vm; - virDomainDeviceDefPtr dev = NULL; - virBitmapPtr qemuCaps = NULL; - virCgroupPtr cgroup = NULL; - int ret = -1; - bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; - - virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_CURRENT | - VIR_DOMAIN_DEVICE_MODIFY_LIVE | - VIR_DOMAIN_DEVICE_MODIFY_CONFIG | - VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1); - - 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) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - qemuReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; - } - - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) - goto cleanup; - - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot attach device on inactive domain")); - goto endjob; - } - - dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, - VIR_DOMAIN_XML_INACTIVE); - if (dev == NULL) - goto endjob; - - if (qemuCapsExtractVersionInfo(vm->def->emulator, vm->def->os.arch, - NULL, - &qemuCaps) < 0) - goto endjob; - - switch (dev->type) { - case VIR_DOMAIN_DEVICE_DISK: - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) { - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) !=0 ) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find cgroup for %s"), - vm->def->name); - goto endjob; - } - if (qemuSetupDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0) - goto endjob; - } - - switch (dev->data.disk->device) { - case VIR_DOMAIN_DISK_DEVICE_CDROM: - case VIR_DOMAIN_DISK_DEVICE_FLOPPY: - ret = qemuDomainChangeEjectableMedia(driver, vm, - dev->data.disk, - qemuCaps, - force); - if (ret == 0) - dev->data.disk = NULL; - break; - - - default: - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk bus '%s' cannot be updated."), - virDomainDiskBusTypeToString(dev->data.disk->bus)); - break; - } - - if (ret != 0 && cgroup) { - if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0) - VIR_WARN("Failed to teardown cgroup for disk path %s", - NULLSTR(dev->data.disk->src)); - } - break; - - case VIR_DOMAIN_DEVICE_GRAPHICS: - ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics); - break; - - default: - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("device type '%s' cannot be updated"), - virDomainDeviceTypeToString(dev->type)); - break; - } - - if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) - ret = -1; - -endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; - -cleanup: - if (cgroup) - virCgroupFree(&cgroup); - - qemuCapsFree(qemuCaps); - virDomainDeviceDefFree(dev); - if (vm) - virDomainObjUnlock(vm); - qemuDriverUnlock(driver); - return ret; + return qemudDomainModifyDeviceFlags(dom, xml, flags, QEMUD_DEVICE_UPDATE); } - - static int qemudDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, unsigned int flags) { -- 1.7.4.1
On 04/21/2011 01:25 AM, KAMEZAWA Hiroyuki wrote:
This patch strips reusable part of qemudDomainUpdateDeviceFlags() and consolidate it to qemudDomainModifyDeviceFlags(). No functional changes.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Changelog: v10->v11 - fixed qemuTeardownDiskCgroup()
- virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | - VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + switch (action) { + case QEMUD_DEVICE_ATTACH: + case QEMUD_DEVICE_DETACH: + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + break; + case QEMUD_DEVICE_UPDATE: + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_CURRENT | + VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG | + VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1); + break; + default: + break;
This lets an invalid action through without warning. And _CURRENT is 0, so it's redundant. I shrunk it a bit. More qemud/qemu renaming. ACK, and pushed with this squashed in: diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index b0ca3ce..771678e 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -3999,11 +3999,11 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, } static int -qemudDomainChangeDiskMediaLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - struct qemud_driver *driver, - virBitmapPtr qemuCaps, - bool force) +qemuDomainChangeDiskMediaLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + struct qemud_driver *driver, + virBitmapPtr qemuCaps, + bool force) { virDomainDiskDefPtr disk = dev->data.disk; virCgroupPtr cgroup = NULL; @@ -4046,18 +4046,19 @@ end: return ret; } -static int qemudDomainUpdateDeviceLive(virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - virDomainPtr dom, - virBitmapPtr qemuCaps, - bool force) +static int +qemuDomainUpdateDeviceLive(virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virDomainPtr dom, + virBitmapPtr qemuCaps, + bool force) { struct qemud_driver *driver = dom->conn->privateData; int ret = -1; switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: - ret = qemudDomainChangeDiskMediaLive(vm, dev, driver, qemuCaps, force); + ret = qemuDomainChangeDiskMediaLive(vm, dev, driver, qemuCaps, force); break; case VIR_DOMAIN_DEVICE_GRAPHICS: ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics); @@ -4091,21 +4092,10 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0; int ret = -1; - switch (action) { - case QEMU_DEVICE_ATTACH: - case QEMU_DEVICE_DETACH: - virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | - VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); - break; - case QEMU_DEVICE_UPDATE: - virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_CURRENT | - VIR_DOMAIN_DEVICE_MODIFY_LIVE | - VIR_DOMAIN_DEVICE_MODIFY_CONFIG | - VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1); - break; - default: - break; - } + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG | + (action == QEMU_DEVICE_UPDATE ? + VIR_DOMAIN_DEVICE_MODIFY_FORCE : 0), -1); if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { qemuReportError(VIR_ERR_OPERATION_INVALID, @@ -4150,9 +4140,11 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, ret = qemuDomainDetachDeviceLive(vm, dev, dom, qemuCaps); break; case QEMU_DEVICE_UPDATE: - ret = qemudDomainUpdateDeviceLive(vm, dev, dom, qemuCaps, force); + ret = qemuDomainUpdateDeviceLive(vm, dev, dom, qemuCaps, force); break; default: + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown domain modify action %d"), action); break; } -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Thu, 21 Apr 2011 15:51:38 -0600 Eric Blake <eblake@redhat.com> wrote:
On 04/21/2011 01:25 AM, KAMEZAWA Hiroyuki wrote:
This patch strips reusable part of qemudDomainUpdateDeviceFlags() and consolidate it to qemudDomainModifyDeviceFlags(). No functional changes.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Changelog: v10->v11 - fixed qemuTeardownDiskCgroup()
- virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | - VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + switch (action) { + case QEMUD_DEVICE_ATTACH: + case QEMUD_DEVICE_DETACH: + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + break; + case QEMUD_DEVICE_UPDATE: + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_CURRENT | + VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG | + VIR_DOMAIN_DEVICE_MODIFY_FORCE, -1); + break; + default: + break;
This lets an invalid action through without warning. And _CURRENT is 0, so it's redundant. I shrunk it a bit.
Ah, yes. I was wrong.
More qemud/qemu renaming.
ACK, and pushed with this squashed in:
Thanks, I'll rework patch 4-6. -Kame
libvirt/qemu support persistent device modification 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> Changelog: v10->v11 - fixed MODIFY_CURRENT case. (virDomainObjCopyPersistentDef): make a copy of persistent vm definition (qemudDomainModifyDeviceFlags): add support for MODIFY_CONFIG and MODIFY_CURRENT (qemudDomainAttach/Detach/UpdateDeviceConfig) : callbacks. now empty (qemudDomainModifyDeviceFlags): modified to handle inactive domain. --- 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 381e692..6c1098a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9509,3 +9509,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 4cba9d0..2c35bbb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4065,6 +4065,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, }; @@ -4075,6 +4117,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; @@ -4083,7 +4126,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 | @@ -4095,12 +4139,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) { @@ -4114,11 +4152,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; + } else { + if (flags == VIR_DOMAIN_DEVICE_MODIFY_CURRENT) + flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG; + /* 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); @@ -4130,33 +4186,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); -- 1.7.4.1
Support changes of disks by VIR_DOMAIN_DEVICE_MODIFY_CONFIG for qemu. This patch includes patches for Attach/Detach. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> * /src/conf/domain_conf.c (virDomainDiskIndexByName): returns array index of disk in vmdef. (virDomainDiskRemoveByName): removes a disk which has the name. * src/qemu/qemu_driver.c (qemudDomainAttachDeviceConfig): add support for Disks. (qemudDomainDetachDeviceConfig): add support for Disks. --- src/conf/domain_conf.c | 22 ++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_driver.c | 36 ++++++++++++++++++++++++++++++++++-- 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6c1098a..d4fac87 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5006,6 +5006,19 @@ virVirtualPortProfileFormat(virBufferPtr buf, virBufferVSprintf(buf, "%s</virtualport>\n", indent); } +int virDomainDiskIndexByName(virDomainDefPtr def, const char *name) +{ + virDomainDiskDefPtr vdisk; + int i; + + for (i = 0; i < def->ndisks; i++) { + vdisk = def->disks[i]; + if (STREQ(vdisk->dst, name)) + return i; + } + return -1; +} + int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk) { @@ -5077,6 +5090,15 @@ void virDomainDiskRemove(virDomainDefPtr def, size_t i) } } +int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name) +{ + int i = virDomainDiskIndexByName(def, name); + if (i < 0) + return -1; + virDomainDiskRemove(def, i); + return 0; +} + int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ddf111a..1dadf98 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1329,6 +1329,7 @@ int virDomainVcpupinAdd(virDomainDefPtr def, int maplen, int vcpu); +int virDomainDiskIndexByName(virDomainDefPtr def, const char *name); int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk); void virDomainDiskInsertPreAlloced(virDomainDefPtr def, @@ -1336,6 +1337,7 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def, int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def); void virDomainDiskRemove(virDomainDefPtr def, size_t i); +int virDomainDiskRemoveByName(virDomainDefPtr def, const char *name); int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f732431..c469259 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -246,11 +246,13 @@ virDomainDiskDefFree; virDomainDiskDeviceTypeToString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; +virDomainDiskIndexByName; virDomainDiskInsert; virDomainDiskInsertPreAlloced; virDomainDiskIoTypeFromString; virDomainDiskIoTypeToString; virDomainDiskRemove; +virDomainDiskRemoveByName; virDomainDiskTypeFromString; virDomainDiskTypeToString; virDomainFSDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2c35bbb..3669f92 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4066,10 +4066,32 @@ static int qemudDomainUpdateDeviceLive(virDomainObjPtr vm, } static int -qemudDomainAttachDeviceConfig(virDomainDefPtr vmdef ATTRIBUTE_UNUSED, +qemudDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { + virDomainDiskDefPtr disk; + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (virDomainDiskIndexByName(vmdef, disk->dst) >= 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s already exists."), disk->dst); + return -1; + } + if (virDomainDiskInsert(vmdef, disk)) { + virReportOOMError(); + return -1; + } + /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ + dev->data.disk = NULL; + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) + if (virDomainDefAddImplicitControllers(vmdef) < 0) + return -1; + if (qemuDomainAssignPCIAddresses(vmdef) < 0) + return -1; + break; + default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("persistent update of device is not supported")); @@ -4080,10 +4102,20 @@ qemudDomainAttachDeviceConfig(virDomainDefPtr vmdef ATTRIBUTE_UNUSED, static int -qemudDomainDetachDeviceConfig(virDomainDefPtr vmdef ATTRIBUTE_UNUSED, +qemudDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { + virDomainDiskDefPtr disk; + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (virDomainDiskRemoveByName(vmdef, disk->dst)) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("no target device %s"), disk->dst); + return -1; + } + break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("persistent update of device is not supported")); -- 1.7.4.1
libvirt/qemu - support update cdrom/floppy persistent. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> * src/qemu/qemu_driver.c (qemudDomainUpdateDeviceConfig): add cdrom/floppy exchange code. --- src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 files changed, 39 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3669f92..ec1fbc6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4125,11 +4125,49 @@ qemudDomainDetachDeviceConfig(virDomainDefPtr vmdef, } static int -qemudDomainUpdateDeviceConfig(virDomainDefPtr vmdef ATTRIBUTE_UNUSED, +qemudDomainUpdateDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev, bool force ATTRIBUTE_UNUSED) { + virDomainDiskDefPtr orig, disk; + int pos; + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + pos = virDomainDiskIndexByName(vmdef, disk->dst); + if (pos < 0) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exists."), disk->dst); + return -1; + } + orig = vmdef->disks[pos]; + if (!(orig->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && + !(orig->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)) { + qemuReportError(VIR_ERR_INVALID_ARG, + _("this disk doesn't support update")); + return -1; + } + /* + * Update 'orig' + * We allow updating src/type//driverType/cachemode/ + */ + VIR_FREE(orig->src); + orig->src = disk->src; + orig->type = disk->type; + orig->cachemode = disk->cachemode; + if (disk->driverName) { + VIR_FREE(orig->driverName); + orig->driverName = disk->driverName; + disk->driverName = NULL; + } + if (disk->driverType) { + VIR_FREE(orig->driverType); + orig->driverType = disk->driverType; + disk->driverType = NULL; + } + disk->src = NULL; + break; default: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("persistent update of device is not supported")); -- 1.7.4.1
I'm assuming you meant for this to go to libvir-list rather than libvirt-users; all my subsequent mails will swap the list reply. On 04/21/2011 01:19 AM, KAMEZAWA Hiroyuki wrote:
Here is v11. Fixed comments/bugs and updated against the latest libvirt.git.
Changes v10->v11: - fixed comments on each patches - fixed cgroup handling in patch 3. - fixed MODIFY_CURRENT handling in patch 4.
most of diff comes from refactoring qemu/qemu_driver.c -- conf/domain_conf.c | 40 ++ conf/domain_conf.h | 5 libvirt_private.syms | 3 qemu/qemu_driver.c | 727 +++++++++++++++++++++++++++++++-------------------- 4 files changed, 500 insertions(+), 275 deletions(-)
Thanks, -Kame
-- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
KAMEZAWA Hiroyuki