[libvirt] [PATCH 1/2] Rename qemudShrinkDisks to virDomainDiskRemove and move to domain_conf.c
From: Soren Hansen <soren@linux2go.dk> Other drivers will need this same functionality, so move it to up to conf/domain_conf.c and give it a more general name. Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 20 ++------------------ 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ee99cd1..e05d5d7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4064,6 +4064,24 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def, } +void virDomainDiskRemove(virDomainDefPtr def, size_t i) +{ + if (def->ndisks > 1) { + memmove(def->disks + i, + def->disks + i + 1, + sizeof(*def->disks) * + (def->ndisks - (i + 1))); + def->ndisks--; + if (VIR_REALLOC_N(def->disks, def->ndisks) < 0) { + /* ignore, harmless */ + } + } else { + VIR_FREE(def->disks); + def->ndisks = 0; + } +} + + int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 92f98bc..7195c04 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1067,6 +1067,8 @@ void virDomainDiskInsertPreAlloced(virDomainDefPtr def, virDomainDiskDefPtr disk); int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def); +void virDomainDiskRemove(virDomainDefPtr def, size_t i); + int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller); void virDomainControllerInsertPreAlloced(virDomainDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d5a7a73..c2905ba 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -144,6 +144,7 @@ virDomainDiskDefFree; virDomainDiskDeviceTypeToString; virDomainDiskInsert; virDomainDiskInsertPreAlloced; +virDomainDiskRemove; virDomainDiskDefAssignAddress; virDomainControllerInsert; virDomainControllerInsertPreAlloced; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 656a1e4..25695df 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8583,22 +8583,6 @@ static inline int qemudFindDisk(virDomainDefPtr def, const char *dst) return -1; } -static inline void qemudShrinkDisks(virDomainDefPtr def, size_t i) -{ - if (def->ndisks > 1) { - memmove(def->disks + i, - def->disks + i + 1, - sizeof(*def->disks) * - (def->ndisks - (i + 1))); - def->ndisks--; - if (VIR_REALLOC_N(def->disks, def->ndisks) < 0) { - /* ignore, harmless */ - } - } else { - VIR_FREE(def->disks); - def->ndisks = 0; - } -} static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, @@ -8655,7 +8639,7 @@ static int qemudDomainDetachPciDiskDevice(struct qemud_driver *driver, qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) VIR_WARN("Unable to release PCI address on %s", dev->data.disk->src); - qemudShrinkDisks(vm->def, i); + virDomainDiskRemove(vm->def, i); virDomainDiskDefFree(detach); @@ -8719,7 +8703,7 @@ static int qemudDomainDetachSCSIDiskDevice(struct qemud_driver *driver, } qemuDomainObjExitMonitorWithDriver(driver, vm); - qemudShrinkDisks(vm->def, i); + virDomainDiskRemove(vm->def, i); virDomainDiskDefFree(detach); -- 1.7.0.4
From: Soren Hansen <soren@linux2go.dk> UML supports hot plugging and unplugging of various devices. This patch exposes this functionality for disks. Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/uml/uml_driver.c | 239 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 235 insertions(+), 4 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 9cad7f1..a5c5d6a 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1683,6 +1683,237 @@ cleanup: } +static int umlDomainAttachUmlDisk(struct uml_driver *driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + int i, ret; + char *cmd = NULL; + char *reply = NULL; + + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (STREQ(vm->def->disks[i]->dst, disk->dst)) { + umlReportError(VIR_ERR_OPERATION_FAILED, + _("target %s already exists"), disk->dst); + return -1; + } + } + + if (!disk->src) { + umlReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("disk source path is missing")); + goto error; + } + + if (virAsprintf(&cmd, "config %s=%s", disk->dst, disk->src) < 0) { + virReportOOMError(); + return -1; + } + + if (umlMonitorCommand(driver, vm, cmd, &reply) < 0) + goto error; + + if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) { + virReportOOMError(); + goto error; + } + + if (ret < 0) + goto error; + + virDomainDiskInsertPreAlloced(vm->def, disk); + + VIR_FREE(reply); + VIR_FREE(cmd); + + return 0; + +error: + + VIR_FREE(reply); + VIR_FREE(cmd); + + return -1; +} + + +static int umlDomainAttachDevice(virDomainPtr dom, const char *xml) +{ + struct uml_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + virDomainDeviceDefPtr dev = NULL; + int ret = -1; + + umlDriverLock(driver); + + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + umlReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + umlReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot attach device on inactive domain")); + goto cleanup; + } + + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + + if (dev == NULL) + goto cleanup; + + if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_UML) { + ret = umlDomainAttachUmlDisk(driver, vm, dev->data.disk); + if (ret == 0) + dev->data.disk = NULL; + } else { + umlReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be hotplugged."), + virDomainDiskBusTypeToString(dev->data.disk->bus)); + } + } else { + umlReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot be attached"), + virDomainDeviceTypeToString(dev->type)); + goto cleanup; + } + +cleanup: + + virDomainDeviceDefFree(dev); + if (vm) + virDomainObjUnlock(vm); + umlDriverUnlock(driver); + return ret; +} + + +static int umlDomainAttachDeviceFlags(virDomainPtr dom, + const char *xml, + unsigned int flags) { + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { + umlReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot modify the persistent configuration of a domain")); + return -1; + } + + return umlDomainAttachDevice(dom, xml); +} + + +static int umlDomainDetachUmlDisk(struct uml_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + int i, ret = -1; + virDomainDiskDefPtr detach = NULL; + char *cmd; + char *reply; + + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) { + break; + } + } + + if (i == vm->def->ndisks) { + umlReportError(VIR_ERR_OPERATION_FAILED, + _("disk %s not found"), dev->data.disk->dst); + return -1; + } + + detach = vm->def->disks[i]; + + if (virAsprintf(&cmd, "remove %s", detach->dst) < 0) { + virReportOOMError(); + return -1; + } + + if (umlMonitorCommand(driver, vm, cmd, &reply) < 0) + goto cleanup; + + virDomainDiskRemove(vm->def, i); + + virDomainDiskDefFree(detach); + + ret = 0; + + VIR_FREE(reply); + +cleanup: + VIR_FREE(cmd); + + return ret; +} + + +static int umlDomainDetachDevice(virDomainPtr dom, const char *xml) { + struct uml_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + virDomainDeviceDefPtr dev = NULL; + int ret = -1; + + umlDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + umlReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + umlReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot detach device on inactive domain")); + goto cleanup; + } + + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto cleanup; + + 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_UML) + ret = umlDomainDetachUmlDisk(driver, vm, dev); + else { + umlReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This type of disk cannot be hot unplugged")); + } + } else { + umlReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("This type of device cannot be hot unplugged")); + } + +cleanup: + virDomainDeviceDefFree(dev); + if (vm) + virDomainObjUnlock(vm); + umlDriverUnlock(driver); + return ret; +} + + +static int umlDomainDetachDeviceFlags(virDomainPtr dom, + const char *xml, + unsigned int flags) { + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { + umlReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot modify the persistent configuration of a domain")); + return -1; + } + + return umlDomainDetachDevice(dom, xml); +} + static int umlDomainGetAutostart(virDomainPtr dom, int *autostart) { @@ -1897,10 +2128,10 @@ static virDriver umlDriver = { umlDomainStartWithFlags, /* domainCreateWithFlags */ umlDomainDefine, /* domainDefineXML */ umlDomainUndefine, /* domainUndefine */ - NULL, /* domainAttachDevice */ - NULL, /* domainAttachDeviceFlags */ - NULL, /* domainDetachDevice */ - NULL, /* domainDetachDeviceFlags */ + umlDomainAttachDevice, /* domainAttachDevice */ + umlDomainAttachDeviceFlags, /* domainAttachDeviceFlags */ + umlDomainDetachDevice, /* domainDetachDevice */ + umlDomainDetachDeviceFlags, /* domainDetachDeviceFlags */ NULL, /* domainUpdateDeviceFlags */ umlDomainGetAutostart, /* domainGetAutostart */ umlDomainSetAutostart, /* domainSetAutostart */ -- 1.7.0.4
On Mon, Aug 23, 2010 at 11:31:27AM +0200, soren@linux2go.dk wrote:
From: Soren Hansen <soren@linux2go.dk>
UML supports hot plugging and unplugging of various devices. This patch exposes this functionality for disks.
Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/uml/uml_driver.c | 239 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 235 insertions(+), 4 deletions(-)
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/8/23 <soren@linux2go.dk>:
From: Soren Hansen <soren@linux2go.dk>
UML supports hot plugging and unplugging of various devices. This patch exposes this functionality for disks.
Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/uml/uml_driver.c | 239 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 235 insertions(+), 4 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 9cad7f1..a5c5d6a 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1683,6 +1683,237 @@ cleanup: }
+static int umlDomainAttachUmlDisk(struct uml_driver *driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + int i, ret; + char *cmd = NULL; + char *reply = NULL; + + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (STREQ(vm->def->disks[i]->dst, disk->dst)) { + umlReportError(VIR_ERR_OPERATION_FAILED, + _("target %s already exists"), disk->dst); + return -1; + } + } + + if (!disk->src) { + umlReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("disk source path is missing")); + goto error; + } + + if (virAsprintf(&cmd, "config %s=%s", disk->dst, disk->src) < 0) { + virReportOOMError(); + return -1; + } + + if (umlMonitorCommand(driver, vm, cmd, &reply) < 0) + goto error; + + if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) { + virReportOOMError(); + goto error; + } + + if (ret < 0) + goto error;
I was about to push this patch, but compile testing gave this error: uml/uml_driver.c: In function 'umlDomainAttachUmlDisk': uml/uml_driver.c:1729: error: 'ret' may be used uninitialized in this function [-Wuninitialized] I think "if (ret < 0) goto error;" and "int ret;" can just be removed completely from this function, but I would like have your ACK on this before doing so. Matthias
On 08/24/2010 02:20 PM, Matthias Bolte wrote:
+static int umlDomainAttachUmlDisk(struct uml_driver *driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + int i, ret; + char *cmd = NULL; + char *reply = NULL; + + for (i = 0 ; i< vm->def->ndisks ; i++) { + if (STREQ(vm->def->disks[i]->dst, disk->dst)) { + umlReportError(VIR_ERR_OPERATION_FAILED, + _("target %s already exists"), disk->dst); + return -1; + } + } + + if (!disk->src) { + umlReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("disk source path is missing")); + goto error; + } + + if (virAsprintf(&cmd, "config %s=%s", disk->dst, disk->src)< 0) { + virReportOOMError(); + return -1; + } + + if (umlMonitorCommand(driver, vm, cmd,&reply)< 0) + goto error; + + if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1)< 0) { + virReportOOMError(); + goto error; + } + + if (ret< 0) + goto error;
I was about to push this patch, but compile testing gave this error:
uml/uml_driver.c: In function 'umlDomainAttachUmlDisk': uml/uml_driver.c:1729: error: 'ret' may be used uninitialized in this function [-Wuninitialized]
I think "if (ret< 0) goto error;" and "int ret;" can just be removed completely from this function, but I would like have your ACK on this before doing so.
ACK to that modification - I don't see any real use of ret in this function. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 24-08-2010 22:20, Matthias Bolte wrote:
+ if (ret < 0) + goto error;
I was about to push this patch, but compile testing gave this error:
uml/uml_driver.c: In function 'umlDomainAttachUmlDisk': uml/uml_driver.c:1729: error: 'ret' may be used uninitialized in this function [-Wuninitialized]
I think "if (ret < 0) goto error;" and "int ret;" can just be removed completely from this function, but I would like have your ACK on this before doing so.
Please do so. I haven't a clue where that came from, and due to the magic of having git rebased every time, I'll never know. :) -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/
2010/8/24 Soren Hansen <soren@ubuntu.com>:
On 24-08-2010 22:20, Matthias Bolte wrote:
+ if (ret < 0) + goto error;
I was about to push this patch, but compile testing gave this error:
uml/uml_driver.c: In function 'umlDomainAttachUmlDisk': uml/uml_driver.c:1729: error: 'ret' may be used uninitialized in this function [-Wuninitialized]
I think "if (ret < 0) goto error;" and "int ret;" can just be removed completely from this function, but I would like have your ACK on this before doing so.
Please do so. I haven't a clue where that came from, and due to the magic of having git rebased every time, I'll never know. :)
Okay, removed the offending code and pushed the result. Matthias
On Mon, Aug 23, 2010 at 11:31:26AM +0200, soren@linux2go.dk wrote:
From: Soren Hansen <soren@linux2go.dk>
Other drivers will need this same functionality, so move it to up to conf/domain_conf.c and give it a more general name.
Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 20 ++------------------ 4 files changed, 23 insertions(+), 18 deletions(-)
ACK, looks good. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/8/23 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Aug 23, 2010 at 11:31:26AM +0200, soren@linux2go.dk wrote:
From: Soren Hansen <soren@linux2go.dk>
Other drivers will need this same functionality, so move it to up to conf/domain_conf.c and give it a more general name.
Signed-off-by: Soren Hansen <soren@linux2go.dk> --- src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 20 ++------------------ 4 files changed, 23 insertions(+), 18 deletions(-)
ACK, looks good.
I applied and pushed this patch. Matthias
participants (5)
-
Daniel P. Berrange -
Eric Blake -
Matthias Bolte -
Soren Hansen -
soren＠linux2go.dk