[libvirt] [PATCH] Support virDomainAttachDevice and virDomainDetachDevice for disks in UML

2010-08-19 Thread soren 
From: Soren Hansen so...@linux2go.dk

UML supports hot plugging and unplugging of various devices. This patch
exposes this functionality for disks.

Signed-off-by: Soren Hansen so...@linux2go.dk
---
 src/uml/uml_driver.c |  223 +-
 1 files changed, 221 insertions(+), 2 deletions(-)

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 04493ba..a09bc60 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1686,6 +1686,225 @@ cleanup:
 }
 
 
+static inline void umlShrinkDisks(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 umlDomainAttachUmlDisk(struct uml_driver *driver,
+  virDomainObjPtr vm,
+  virDomainDiskDefPtr disk)
+{
+int i, ret;
+char *cmd = NULL;
+char *reply;
+
+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)
+return -1;
+
+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(cmd);
+
+return 0;
+
+error:
+
+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 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;
+
+umlShrinkDisks(vm-def, i);
+
+virDomainDiskDefFree(detach);
+
+ret = 0;
+
+cleanup:
+VIR_FREE(cmd);
+
+return ret;
+}
+
+
+static int umlDomainDetachDevice(virDomainPtr dom, const char *xml) {
+struct uml_driver *driver =

Re: [libvirt] [PATCH] Support virDomainAttachDevice and virDomainDetachDevice for disks in UML

2010-08-19 Thread Daniel P. Berrange 
On Thu, Aug 19, 2010 at 02:49:34PM +0200, so...@linux2go.dk wrote:
 From: Soren Hansen so...@linux2go.dk
 
 UML supports hot plugging and unplugging of various devices. This patch
 exposes this functionality for disks.

Nice, I had no idea they supported that.

 Signed-off-by: Soren Hansen so...@linux2go.dk
 ---
  src/uml/uml_driver.c |  223 
 +-
  1 files changed, 221 insertions(+), 2 deletions(-)
 
 diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
 index 04493ba..a09bc60 100644
 --- a/src/uml/uml_driver.c
 +++ b/src/uml/uml_driver.c
 @@ -1686,6 +1686,225 @@ cleanup:
  }
  
  
 +static inline void umlShrinkDisks(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;
 +}
 +}

Since this code is already used in the QEMU driver, I think we
should just put it straight into src/conf/domain_conf.c so we
can share it. 'virDomainDiskRemove' is probably a more accurate
name than ShrinkDisks.


 +
 +
 +static int umlDomainAttachUmlDisk(struct uml_driver *driver,
 +  virDomainObjPtr vm,
 +  virDomainDiskDefPtr disk)
 +{
 +int i, ret;
 +char *cmd = NULL;
 +char *reply;
 +
 +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)
 +return -1;

Needs to be a 'goto error' to avoid leaking 'cmd'

 +
 +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(cmd);
 +
 +return 0;
 +
 +error:
 +
 +VIR_FREE(cmd);
 +
 +return -1;
 +}

 +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 umlDomainGetAutostart(virDomainPtr dom,
  int *autostart) {
 @@ -1900,9 +2119,9 @@ static virDriver umlDriver = {
  umlDomainStartWithFlags, /* domainCreateWithFlags */
  umlDomainDefine, /* domainDefineXML */
  umlDomainUndefine, /* domainUndefine */
 -NULL, /* domainAttachDevice */
 +umlDomainAttachDevice, /* domainAttachDevice */
  NULL, /* domainAttachDeviceFlags */
 -NULL, /* domainDetachDevice */
 +umlDomainDetachDevice, /* domainDetachDevice */
  NULL, /* domainDetachDeviceFlags */

You should also implement the DeviceFlags variants at
the same time. You can just do a dumb wrapper like QEMU
driver does, for simplicity.

Regards,
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Support virDomainAttachDevice and virDomainDetachDevice for disks in UML

2010-08-19 Thread Soren Hansen 
On 19-08-2010 15:22, Daniel P. Berrange wrote:
 +static inline void umlShrinkDisks(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;
 +}
 +}
 Since this code is already used in the QEMU driver, I think we
 should just put it straight into src/conf/domain_conf.c so we
 can share it. 'virDomainDiskRemove' is probably a more accurate
 name than ShrinkDisks.

Sounds good to me. I wasn't sure where to stick it, so I just left it
here and figured you'd probably tell me something like this :) I'll move it.

 +
 +
 +static int umlDomainAttachUmlDisk(struct uml_driver *driver,
 +  virDomainObjPtr vm,
 +  virDomainDiskDefPtr disk)
 +{
 +int i, ret;
 +char *cmd = NULL;
 +char *reply;
 +
 +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)
 +return -1;
 
 Needs to be a 'goto error' to avoid leaking 'cmd'

Ah, yes, good catch. I also need to free the reply. :)

 @@ -1900,9 +2119,9 @@ static virDriver umlDriver = {
  umlDomainStartWithFlags, /* domainCreateWithFlags */
  umlDomainDefine, /* domainDefineXML */
  umlDomainUndefine, /* domainUndefine */
 -NULL, /* domainAttachDevice */
 +umlDomainAttachDevice, /* domainAttachDevice */
  NULL, /* domainAttachDeviceFlags */
 -NULL, /* domainDetachDevice */
 +umlDomainDetachDevice, /* domainDetachDevice */
  NULL, /* domainDetachDeviceFlags */
 
 You should also implement the DeviceFlags variants at
 the same time. You can just do a dumb wrapper like QEMU
 driver does, for simplicity.

Right you are. I misunderstood what those were for.

-- 
Soren Hansen
Ubuntu Developer
http://www.ubuntu.com/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list