[libvirt] [PATCHv10 0/6] libvirt - support persistent device modification
Hi, this is v10, totally refleshed. Thank you for advices. This version includes Eric's cleanup and Hu's Update device works. (And droppped some sanity check patches.) Because of many changes from v9, it may be better to write [RFC] in subject ;) This patch series does consolidate Attach/Detach/Update device's codes as.. == shared code (prepare lock, get objects etc..) switch(action) { case ATTACH case DETACH case UPDATE } shared code. == After that, adding persistent modification support as == shared code (prepare lock, get objects etc...) if (MODIFY_CONFIG) { switch (action) { case ATTACH: case DETACH: case UPDATE: } } if (MODIFY_LIVE) { switch (action) { case ATTACH: case DETACH: case UPDATE: } } shared code. (save config etc.) == 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, and rename... (qemuDomainAttachDevice, qemuDomainAttachDeviceFlags): to this. (qemudDomainDetachDevice, qemudDomainDetachDeviceFlags): --- 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
At 04/19/2011 03:40 PM, KAMEZAWA Hiroyuki Write:
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, and rename...
Hmm, you do not rename the function(qemudDomainAttachDevice ===> qemuDomainAttachDevice)
(qemuDomainAttachDevice, qemuDomainAttachDeviceFlags): to this.
Hmm, it may be update not attach here.
(qemudDomainDetachDevice, qemudDomainDetachDeviceFlags):
Missing '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,
On Tue, 19 Apr 2011 17:29:08 +0800 Wen Congyang <wency@cn.fujitsu.com> wrote:
At 04/19/2011 03:40 PM, KAMEZAWA Hiroyuki Write:
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, and rename...
Hmm, you do not rename the function(qemudDomainAttachDevice ===> qemuDomainAttachDevice)
typo
(qemuDomainAttachDevice, qemuDomainAttachDeviceFlags): to this.
Hmm, it may be update not attach here.
typo
(qemudDomainDetachDevice, qemudDomainDetachDeviceFlags):
Missing 'Likewise.'
? Thanks, -Kame
--- 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,
Fixed typos in commit comments. ==
From 58c8e35fbadce5a7b085045fec41b0bf57ae9a92 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Date: Tue, 19 Apr 2011 13:21:57 +0900 Subject: [PATCHv10 1/6] libvirt/qemu - clean up for qemudDomainAt(de)tachDevice(Flags)
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, and 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
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. From: Eric Blake <eblake@redhat.com> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> * src/qemu/qemu_driver.c (qemudDomainAttachDeviceFlags) : pulled out to qemudDomainModifyDeviceFlags() (qemudDomainModifyDeviceFlags) : impelements a generic codes for modify domain. (qemudDomainAttachDeviceFlagsLive) : codes for attaching devices to domain in live, no changes in logic from old code. (qemudDomainDetachDeviceFlagsLive) : codes for detaching devices to domain in live, no changes in logic from old code. --- 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
At 04/19/2011 03:43 PM, KAMEZAWA Hiroyuki Write:
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.
From: Eric Blake <eblake@redhat.com> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
* src/qemu/qemu_driver.c (qemudDomainAttachDeviceFlags) : pulled out to qemudDomainModifyDeviceFlags() (qemudDomainModifyDeviceFlags) : impelements a generic codes for modify domain. (qemudDomainAttachDeviceFlagsLive) : codes for attaching devices to domain in live, no changes in logic from old code. (qemudDomainDetachDeviceFlagsLive) : codes for detaching devices to domain in live, no changes in logic from old code.
You introduce some functions qemudDomainAttachXXXLive() and qemudDomainDetachXXXLive() to make qemudDomainAttachDeviceFlags() and qemudDomainDetachDeviceFlags() to be small. You should log them in commit messages.
--- src/qemu/qemu_driver.c | 429 ++++++++++++++++++++++++++---------------------- 1 files changed, 234 insertions(+), 195 deletions(-)
This patch strips reusable part of qemudDomainUpdateDeviceFlags() and consolidate it to qemudDomainModifyDeviceFlags(). No functional changes. Based on Eric's and Hu's work. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> * src/qemu/qemu_driver.c (qemudDomainUpdateDeviceLive) : core of UpdateDevice, extracted from UpdateDeviceFlags() (qemudDomainUpdateDeviceFlags): reworked as a wrapper function of ModifyDeviceFlags() --- src/qemu/qemu_driver.c | 209 +++++++++++++++++++++--------------------------- 1 files changed, 90 insertions(+), 119 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f33a7f4..2bdf42e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3991,6 +3991,74 @@ 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; + } +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 +4070,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 +4131,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 +4176,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
At 04/19/2011 03:44 PM, KAMEZAWA Hiroyuki Write:
This patch strips reusable part of qemudDomainUpdateDeviceFlags() and consolidate it to qemudDomainModifyDeviceFlags(). No functional changes. Based on Eric's and Hu's work.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
* src/qemu/qemu_driver.c (qemudDomainUpdateDeviceLive) : core of UpdateDevice, extracted from UpdateDeviceFlags() (qemudDomainUpdateDeviceFlags): reworked as a wrapper function of ModifyDeviceFlags() --- src/qemu/qemu_driver.c | 209 +++++++++++++++++++++--------------------------- 1 files changed, 90 insertions(+), 119 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f33a7f4..2bdf42e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3991,6 +3991,74 @@ 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; + } +end:
If ret is not 0 and cgroup is not NULL, you should call qemuTeardownDiskCgroup() to do some cleanup.
+ if (cgroup) + virCgroupFree(&cgroup); + return ret; +} +
On Wed, 20 Apr 2011 10:34:13 +0800 Wen Congyang <wency@cn.fujitsu.com> wrote:
At 04/19/2011 03:44 PM, KAMEZAWA Hiroyuki Write:
This patch strips reusable part of qemudDomainUpdateDeviceFlags() and consolidate it to qemudDomainModifyDeviceFlags(). No functional changes. Based on Eric's and Hu's work.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
* src/qemu/qemu_driver.c (qemudDomainUpdateDeviceLive) : core of UpdateDevice, extracted from UpdateDeviceFlags() (qemudDomainUpdateDeviceFlags): reworked as a wrapper function of ModifyDeviceFlags() --- src/qemu/qemu_driver.c | 209 +++++++++++++++++++++--------------------------- 1 files changed, 90 insertions(+), 119 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f33a7f4..2bdf42e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3991,6 +3991,74 @@ 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; + } +end:
If ret is not 0 and cgroup is not NULL, you should call qemuTeardownDiskCgroup() to do some cleanup.
Ah, yes. it's mistake. Will fix in v11. Thanks, -Kame
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; + } 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); @@ -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); -- 1.7.4.1
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);
On Wed, 20 Apr 2011 10:52:41 +0800 Wen Congyang <wency@cn.fujitsu.com> 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@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'.
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
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 bb8f0a4..43e9330 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5007,6 +5007,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) { @@ -5078,6 +5091,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 4ac8f7e..92643ef 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4060,10 +4060,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")); @@ -4074,10 +4096,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
This patch adds support for Disk exchange in 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 92643ef..6188722 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4119,11 +4119,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
participants (2)
-
KAMEZAWA Hiroyuki -
Wen Congyang