[libvirt] [PATCH 0/3] Add device disk attach/detach support to libxl driver

This patch series adds the infrastructure to support attaching and detaching of devices to the libxl driver. Currently only disk devices are supported. The first patch refactors some functions that are needed to create the appropriate datastructures for the libxenlight interface. The second patch fixes the handling of vm def's on cleanup. The last patch finally adds the device attach/detach code and follows qemu's implementation. Markus Groß (3): Refactored libxl datastructure instantiation Fix libxl vm def handling on domU cleanup Add disk attach/detach support to libxl driver src/libxl/libxl_conf.c | 304 ++++++++++++++------------- src/libxl/libxl_conf.h | 8 + src/libxl/libxl_driver.c | 526 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 697 insertions(+), 141 deletions(-) -- 1.7.5.1

The new public functions can be used for attaching/detaching devices. --- src/libxl/libxl_conf.c | 304 ++++++++++++++++++++++++++---------------------- src/libxl/libxl_conf.h | 8 ++ 2 files changed, 171 insertions(+), 141 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 3cebf41..5ae8494 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1,5 +1,6 @@ /*---------------------------------------------------------------------------*/ /* Copyright (c) 2011 SUSE LINUX Products GmbH, Nuernberg, Germany. + * Copyright (C) 2011 Univention GmbH. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -17,6 +18,7 @@ * * Authors: * Jim Fehlig <jfehlig@novell.com> + * Markus Groß <gross@univention.de> */ /*---------------------------------------------------------------------------*/ @@ -461,6 +463,68 @@ error: return -1; } +int +libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) +{ + if (l_disk->src && (x_disk->pdev_path = strdup(l_disk->src)) == NULL) { + virReportOOMError(); + return -1; + } + + if (l_disk->dst && (x_disk->vdev = strdup(l_disk->dst)) == NULL) { + virReportOOMError(); + return -1; + } + + if (l_disk->driverName) { + if (STREQ(l_disk->driverName, "tap") || + STREQ(l_disk->driverName, "tap2")) { + if (l_disk->driverType) { + if (STREQ(l_disk->driverType, "qcow")) { + x_disk->format = DISK_FORMAT_QCOW; + x_disk->backend = DISK_BACKEND_QDISK; + } else if (STREQ(l_disk->driverType, "qcow2")) { + x_disk->format = DISK_FORMAT_QCOW2; + x_disk->backend = DISK_BACKEND_QDISK; + } else if (STREQ(l_disk->driverType, "vhd")) { + x_disk->format = DISK_FORMAT_VHD; + x_disk->backend = DISK_BACKEND_TAP; + } else if (STREQ(l_disk->driverType, "aio") || + STREQ(l_disk->driverType, "raw")) { + x_disk->format = DISK_FORMAT_RAW; + x_disk->backend = DISK_BACKEND_TAP; + } + } else { + /* No subtype specified, default to raw/tap */ + x_disk->format = DISK_FORMAT_RAW; + x_disk->backend = DISK_BACKEND_TAP; + } + } else if (STREQ(l_disk->driverName, "file")) { + x_disk->format = DISK_FORMAT_RAW; + x_disk->backend = DISK_BACKEND_TAP; + } else if (STREQ(l_disk->driverName, "phy")) { + x_disk->format = DISK_FORMAT_RAW; + x_disk->backend = DISK_BACKEND_PHY; + } else { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight does not support disk driver %s"), + l_disk->driverName); + return -1; + } + } else { + /* No driverName - default to raw/tap?? */ + x_disk->format = DISK_FORMAT_RAW; + x_disk->backend = DISK_BACKEND_TAP; + } + + /* How to set unpluggable? */ + x_disk->unpluggable = 1; + x_disk->readwrite = !l_disk->readonly; + x_disk->is_cdrom = l_disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? 1 : 0; + + return 0; +} + static int libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) { @@ -475,64 +539,8 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) } for (i = 0; i < ndisks; i++) { - if (l_disks[i]->src && - (x_disks[i].pdev_path = strdup(l_disks[i]->src)) == NULL) { - virReportOOMError(); + if (libxlMakeDisk(l_disks[i], &x_disks[i]) < 0) goto error; - } - - if (l_disks[i]->dst && - (x_disks[i].vdev = strdup(l_disks[i]->dst)) == NULL) { - virReportOOMError(); - goto error; - } - - if (l_disks[i]->driverName) { - if (STREQ(l_disks[i]->driverName, "tap") || - STREQ(l_disks[i]->driverName, "tap2")) { - if (l_disks[i]->driverType) { - if (STREQ(l_disks[i]->driverType, "qcow")) { - x_disks[i].format = DISK_FORMAT_QCOW; - x_disks[i].backend = DISK_BACKEND_QDISK; - } else if (STREQ(l_disks[i]->driverType, "qcow2")) { - x_disks[i].format = DISK_FORMAT_QCOW2; - x_disks[i].backend = DISK_BACKEND_QDISK; - } else if (STREQ(l_disks[i]->driverType, "vhd")) { - x_disks[i].format = DISK_FORMAT_VHD; - x_disks[i].backend = DISK_BACKEND_TAP; - } else if (STREQ(l_disks[i]->driverType, "aio") || - STREQ(l_disks[i]->driverType, "raw")) { - x_disks[i].format = DISK_FORMAT_RAW; - x_disks[i].backend = DISK_BACKEND_TAP; - } - } else { - /* No subtype specified, default to raw/tap */ - x_disks[i].format = DISK_FORMAT_RAW; - x_disks[i].backend = DISK_BACKEND_TAP; - } - } else if (STREQ(l_disks[i]->driverName, "file")) { - x_disks[i].format = DISK_FORMAT_RAW; - x_disks[i].backend = DISK_BACKEND_TAP; - } else if (STREQ(l_disks[i]->driverName, "phy")) { - x_disks[i].format = DISK_FORMAT_RAW; - x_disks[i].backend = DISK_BACKEND_PHY; - } else { - libxlError(VIR_ERR_INTERNAL_ERROR, - _("libxenlight does not support disk driver %s"), - l_disks[i]->driverName); - goto error; - } - } else { - /* No driverName - default to raw/tap?? */ - x_disks[i].format = DISK_FORMAT_RAW; - x_disks[i].backend = DISK_BACKEND_TAP; - } - - /* How to set unpluggable? */ - x_disks[i].unpluggable = 1; - x_disks[i].readwrite = !l_disks[i]->readonly; - x_disks[i].is_cdrom = - l_disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? 1 : 0; } d_config->disks = x_disks; @@ -547,6 +555,45 @@ error: return -1; } +int +libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic) +{ + // TODO: Where is mtu stored? + //x_nics[i].mtu = 1492; + + memcpy(x_nic->mac, l_nic->mac, sizeof(libxl_mac)); + + if (l_nic->model && !STREQ(l_nic->model, "netfront")) { + if ((x_nic->model = strdup(l_nic->model)) == NULL) { + virReportOOMError(); + return -1; + } + x_nic->nictype = NICTYPE_IOEMU; + } else { + x_nic->nictype = NICTYPE_VIF; + } + + if (l_nic->ifname && (x_nic->ifname = strdup(l_nic->ifname)) == NULL) { + virReportOOMError(); + return -1; + } + + if (l_nic->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { + if (l_nic->data.bridge.brname && + (x_nic->bridge = strdup(l_nic->data.bridge.brname)) == NULL) { + virReportOOMError(); + return -1; + } + if (l_nic->data.bridge.script && + (x_nic->script = strdup(l_nic->data.bridge.script)) == NULL) { + virReportOOMError(); + return -1; + } + } + + return 0; +} + static int libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) { @@ -563,41 +610,8 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) for (i = 0; i < nnics; i++) { x_nics[i].devid = i; - // TODO: Where is mtu stored? - //x_nics[i].mtu = 1492; - - memcpy(x_nics[i].mac, l_nics[i]->mac, sizeof(libxl_mac)); - - if (l_nics[i]->model && !STREQ(l_nics[i]->model, "netfront")) { - if ((x_nics[i].model = strdup(l_nics[i]->model)) == NULL) { - virReportOOMError(); - goto error; - } - x_nics[i].nictype = NICTYPE_IOEMU; - } else { - x_nics[i].nictype = NICTYPE_VIF; - } - - if (l_nics[i]->ifname && - (x_nics[i].ifname = strdup(l_nics[i]->ifname)) == NULL) { - virReportOOMError(); + if (libxlMakeNic(l_nics[i], &x_nics[i])) goto error; - } - - if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { - if (l_nics[i]->data.bridge.brname && - (x_nics[i].bridge = - strdup(l_nics[i]->data.bridge.brname)) == NULL) { - virReportOOMError(); - goto error; - } - if (l_nics[i]->data.bridge.script && - (x_nics[i].script = - strdup(l_nics[i]->data.bridge.script)) == NULL) { - virReportOOMError(); - goto error; - } - } } d_config->vifs = x_nics; @@ -612,6 +626,62 @@ error: return -1; } +int +libxlMakeVfb(libxlDriverPrivatePtr driver, virDomainGraphicsDefPtr l_vfb, + libxl_device_vfb *x_vfb) +{ + int port; + + switch (l_vfb->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + x_vfb->sdl = 1; + if (l_vfb->data.sdl.display && + (x_vfb->display = strdup(l_vfb->data.sdl.display)) == NULL) { + virReportOOMError(); + return -1; + } + if (l_vfb->data.sdl.xauth && + (x_vfb->xauthority = + strdup(l_vfb->data.sdl.xauth)) == NULL) { + virReportOOMError(); + return -1; + } + break; + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + x_vfb->vnc = 1; + /* driver handles selection of free port */ + x_vfb->vncunused = 0; + if (l_vfb->data.vnc.autoport) { + port = libxlNextFreeVncPort(driver, LIBXL_VNC_PORT_MIN); + if (port < 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unable to find an unused VNC port")); + return -1; + } + l_vfb->data.vnc.port = port; + } + x_vfb->vncdisplay = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN; + + if (l_vfb->data.vnc.listenAddr) { + /* libxl_device_vfb_init() does strdup("127.0.0.1") */ + free(x_vfb->vnclisten); + if ((x_vfb->vnclisten = + strdup(l_vfb->data.vnc.listenAddr)) == NULL) { + virReportOOMError(); + return -1; + } + } + if (l_vfb->data.vnc.keymap && + (x_vfb->keymap = + strdup(l_vfb->data.vnc.keymap)) == NULL) { + virReportOOMError(); + return -1; + } + break; + } + return 0; +} + static int libxlMakeVfbList(libxlDriverPrivatePtr driver, virDomainDefPtr def, libxl_domain_config *d_config) @@ -621,7 +691,6 @@ libxlMakeVfbList(libxlDriverPrivatePtr driver, libxl_device_vfb *x_vfbs; libxl_device_vkb *x_vkbs; int i; - int port; if (nvfbs == 0) return 0; @@ -640,55 +709,8 @@ libxlMakeVfbList(libxlDriverPrivatePtr driver, libxl_device_vfb_init(&x_vfbs[i], i); libxl_device_vkb_init(&x_vkbs[i], i); - switch (l_vfbs[i]->type) { - case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - x_vfbs[i].sdl = 1; - if (l_vfbs[i]->data.sdl.display && - (x_vfbs[i].display = - strdup(l_vfbs[i]->data.sdl.display)) == NULL) { - virReportOOMError(); - goto error; - } - if (l_vfbs[i]->data.sdl.xauth && - (x_vfbs[i].xauthority = - strdup(l_vfbs[i]->data.sdl.xauth)) == NULL) { - virReportOOMError(); - goto error; - } - break; - case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - x_vfbs[i].vnc = 1; - /* driver handles selection of free port */ - x_vfbs[i].vncunused = 0; - if (l_vfbs[i]->data.vnc.autoport) { - port = libxlNextFreeVncPort(driver, LIBXL_VNC_PORT_MIN); - if (port < 0) { - libxlError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unable to find an unused VNC port")); - goto error; - } - l_vfbs[i]->data.vnc.port = port; - } - x_vfbs[i].vncdisplay = l_vfbs[i]->data.vnc.port - - LIBXL_VNC_PORT_MIN; - - if (l_vfbs[i]->data.vnc.listenAddr) { - /* libxl_device_vfb_init() does strdup("127.0.0.1") */ - free(x_vfbs[i].vnclisten); - if ((x_vfbs[i].vnclisten = - strdup(l_vfbs[i]->data.vnc.listenAddr)) == NULL) { - virReportOOMError(); - goto error; - } - } - if (l_vfbs[i]->data.vnc.keymap && - (x_vfbs[i].keymap = - strdup(l_vfbs[i]->data.vnc.keymap)) == NULL) { - virReportOOMError(); - goto error; - } - break; - } + if (libxlMakeVfb(driver, l_vfbs[i], &x_vfbs[i]) < 0) + goto error; } d_config->vfbs = x_vfbs; diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 65110cf..5707b4e 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -90,6 +90,14 @@ virCapsPtr libxlMakeCapabilities(libxl_ctx *ctx); int +libxlMakeDisk(virDomainDiskDefPtr l_dev, libxl_device_disk *x_dev); +int +libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic); +int +libxlMakeVfb(libxlDriverPrivatePtr driver, virDomainGraphicsDefPtr l_vfb, + libxl_device_vfb *x_vfb); + +int libxlBuildDomainConfig(libxlDriverPrivatePtr driver, virDomainDefPtr def, libxl_domain_config *d_config); -- 1.7.5.1

Otherwise vm->newDef remains in the domain object even when it is destroyed. --- src/libxl/libxl_driver.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 5cc9362..a14ace1 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -276,6 +276,13 @@ libxlVmCleanup(libxlDriverPrivatePtr driver, VIR_DEBUG("Failed to remove domain XML for %s", vm->def->name); VIR_FREE(file); } + + if (vm->newDef) { + virDomainDefFree(vm->def); + vm->def = vm->newDef; + vm->def->id = -1; + vm->newDef = NULL; + } } /* -- 1.7.5.1

Based on the device attach/detach code from the QEMU driver. --- src/libxl/libxl_driver.c | 519 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 519 insertions(+), 0 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a14ace1..a056be9 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2219,6 +2219,520 @@ libxlDomainUndefine(virDomainPtr dom) return ret; } +static int +libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, virDomainDiskDefPtr disk) +{ + virDomainDiskDefPtr origdisk = NULL; + libxl_device_disk x_disk; + int i; + int ret = -1; + + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (vm->def->disks[i]->bus == disk->bus && + STREQ(vm->def->disks[i]->dst, disk->dst)) { + origdisk = vm->def->disks[i]; + break; + } + } + + if (!origdisk) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("No device with bus '%s' and target '%s'"), + virDomainDiskBusTypeToString(disk->bus), disk->dst); + goto cleanup; + } + + if (origdisk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("Removable media not supported for %s device"), + virDomainDiskDeviceTypeToString(disk->device)); + return -1; + } + + if (libxlMakeDisk(disk, &x_disk) < 0) + goto cleanup; + + if ((ret = libxl_cdrom_insert(&priv->ctx, vm->def->id, &x_disk)) < 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight failed to change media for disk '%s'"), + disk->dst); + goto cleanup; + } + + VIR_FREE(origdisk->src); + origdisk->src = disk->src; + disk->src = NULL; + origdisk->type = disk->type; + + + virDomainDiskDefFree(disk); + + ret = 0; + +cleanup: + return ret; +} + +static int +libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr l_disk = dev->data.disk; + libxl_device_disk x_disk; + int ret = -1; + + switch (l_disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + ret = libxlDomainChangeEjectableMedia(priv, vm, l_disk); + break; + case VIR_DOMAIN_DISK_DEVICE_DISK: + if (l_disk->bus == VIR_DOMAIN_DISK_BUS_XEN) { + if (virDomainDiskIndexByName(vm->def, l_disk->dst) >= 0) { + libxlError(VIR_ERR_OPERATION_FAILED, + _("target %s already exists"), l_disk->dst); + goto cleanup; + } + + if (!l_disk->src) { + libxlError(VIR_ERR_INTERNAL_ERROR, + "%s", _("disk source path is missing")); + goto cleanup; + } + + if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (libxlMakeDisk(l_disk, &x_disk) < 0) + goto cleanup; + + if ((ret = libxl_device_disk_add(&priv->ctx, vm->def->id, + &x_disk)) < 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight failed to attach disk '%s'"), + l_disk->dst); + goto cleanup; + } + + virDomainDiskInsertPreAlloced(vm->def, l_disk); + + } else { + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be hotplugged."), + virDomainDiskBusTypeToString(l_disk->bus)); + } + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk device type '%s' cannot be hotplugged"), + virDomainDiskDeviceTypeToString(l_disk->device)); + break; + } + +cleanup: + return ret; +} + +static int +libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr l_disk = NULL; + libxl_device_disk x_disk; + int i; + int wait_secs = 2; + int ret = -1; + + switch (dev->data.disk->device) { + case VIR_DOMAIN_DISK_DEVICE_DISK: + if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_XEN) { + + if ((i = virDomainDiskIndexByName(vm->def, + dev->data.disk->dst)) < 0) { + libxlError(VIR_ERR_OPERATION_FAILED, + _("disk %s not found"), dev->data.disk->dst); + goto cleanup; + } + + l_disk = vm->def->disks[i]; + + if (libxlMakeDisk(l_disk, &x_disk) < 0) + goto cleanup; + + if ((ret = libxl_device_disk_del(&priv->ctx, &x_disk, + wait_secs)) < 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight failed to detach disk '%s'"), + l_disk->dst); + goto cleanup; + } + + virDomainDiskRemove(vm->def, i); + virDomainDiskDefFree(l_disk); + + } else { + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be hot unplugged."), + virDomainDiskBusTypeToString(l_disk->bus)); + } + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot hot unplugged"), + virDomainDiskDeviceTypeToString(dev->data.disk->device)); + break; + } + +cleanup: + return ret; +} + +static int +libxlDomainAttachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + ret = libxlDomainAttachDeviceDiskLive(priv, vm, dev); + if (!ret) + dev->data.disk = NULL; + break; + + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot be attached"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} + +static int +libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (virDomainDiskIndexByName(vmdef, disk->dst) >= 0) { + libxlError(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; + break; + + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("persistent attach of device is not supported")); + return -1; + } + return 0; +} + +static int +libxlDomainDetachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + ret = libxlDomainDetachDeviceDiskLive(priv, vm, dev); + break; + + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot be detached"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} + +static int +libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (virDomainDiskRemoveByName(vmdef, disk->dst)) { + libxlError(VIR_ERR_INVALID_ARG, + _("no target device %s"), disk->dst); + break; + } + ret = 0; + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("persistent detach of device is not supported")); + break; + } + + return ret; +} + +static int +libxlDomainUpdateDeviceLive(libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + switch (disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + ret = libxlDomainChangeEjectableMedia(priv, vm, disk); + if (ret == 0) + dev->data.disk = NULL; + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be updated."), + virDomainDiskBusTypeToString(disk->bus)); + break; + } + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot be updated"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} + +static int +libxlDomainUpdateDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr orig; + virDomainDiskDefPtr disk; + int i; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if ((i = virDomainDiskIndexByName(vmdef, disk->dst)) < 0) { + libxlError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exists."), disk->dst); + goto cleanup; + } + orig = vmdef->disks[i]; + if (!(orig->device == VIR_DOMAIN_DISK_DEVICE_CDROM)) { + libxlError(VIR_ERR_INVALID_ARG, + _("this disk doesn't support update")); + goto cleanup; + } + + VIR_FREE(orig->src); + orig->src = disk->src; + orig->type = disk->type; + 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: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("persistent update of device is not supported")); + goto cleanup; + } + + ret = 0; + +cleanup: + return ret; +} + +/* Actions for libxlDomainModifyDeviceFlags */ +enum { + LIBXL_DEVICE_ATTACH, + LIBXL_DEVICE_DETACH, + LIBXL_DEVICE_UPDATE, +}; + +static int +libxlDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags, int action) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainDefPtr vmdef = NULL; + virDomainDeviceDefPtr dev = NULL; + libxlDomainObjPrivatePtr priv; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + + libxlDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (!vm) { + libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching uuid")); + goto cleanup; + } + + 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) { + libxlError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is not running")); + goto cleanup; + } + } + + if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) { + libxlError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot modify device on transient domain")); + goto cleanup; + } + + if (!(dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + priv = vm->privateData; + + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { + if (!(dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + /* Make a copy for updated domain. */ + if (!(vmdef = virDomainObjCopyPersistentDef(driver->caps, vm))) + goto cleanup; + + switch (action) { + case LIBXL_DEVICE_ATTACH: + ret = libxlDomainAttachDeviceConfig(vmdef, dev); + break; + case LIBXL_DEVICE_DETACH: + ret = libxlDomainDetachDeviceConfig(vmdef, dev); + break; + case LIBXL_DEVICE_UPDATE: + ret = libxlDomainUpdateDeviceConfig(vmdef, dev); + default: + libxlError(VIR_ERR_INTERNAL_ERROR, + _("unknown domain modify action %d"), action); + break; + } + } else + ret = 0; + + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { + /* If dev exists it was created to modify the domain config. Free it, */ + virDomainDeviceDefFree(dev); + if (!(dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + switch (action) { + case LIBXL_DEVICE_ATTACH: + ret = libxlDomainAttachDeviceLive(priv, vm, dev); + break; + case LIBXL_DEVICE_DETACH: + ret = libxlDomainDetachDeviceLive(priv, vm, dev); + break; + case LIBXL_DEVICE_UPDATE: + ret = libxlDomainUpdateDeviceLive(priv, vm, dev); + default: + libxlError(VIR_ERR_INTERNAL_ERROR, + _("unknown domain modify action %d"), action); + break; + } + /* + * update domain status forcibly because the domain status may be + * changed even if we attach the device failed. + */ + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) + ret = -1; + } + + /* Finally, if no error until here, we can save config. */ + if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) { + ret = virDomainSaveConfig(driver->configDir, vmdef); + if (!ret) { + virDomainObjAssignDef(vm, vmdef, false); + vmdef = NULL; + } + } + +cleanup: + virDomainDefFree(vmdef); + virDomainDeviceDefFree(dev); + if (vm) + virDomainObjUnlock(vm); + libxlDriverUnlock(driver); + return ret; +} + +static int +libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) +{ + return libxlDomainModifyDeviceFlags(dom, xml, flags, LIBXL_DEVICE_ATTACH); +} + +static int +libxlDomainAttachDevice(virDomainPtr dom, const char *xml) +{ + return libxlDomainAttachDeviceFlags(dom, xml, + VIR_DOMAIN_DEVICE_MODIFY_LIVE); +} + +static int +libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) +{ + return libxlDomainModifyDeviceFlags(dom, xml, flags, LIBXL_DEVICE_DETACH); +} + +static int +libxlDomainDetachDevice(virDomainPtr dom, const char *xml) +{ + return libxlDomainDetachDeviceFlags(dom, xml, + VIR_DOMAIN_DEVICE_MODIFY_LIVE); +} + +static int +libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) +{ + return libxlDomainModifyDeviceFlags(dom, xml, flags, LIBXL_DEVICE_UPDATE); +} + static unsigned long long libxlNodeGetFreeMemory(virConnectPtr conn) { @@ -2736,6 +3250,11 @@ static virDriver libxlDriver = { .domainCreateWithFlags = libxlDomainCreateWithFlags, /* 0.9.0 */ .domainDefineXML = libxlDomainDefineXML, /* 0.9.0 */ .domainUndefine = libxlDomainUndefine, /* 0.9.0 */ + .domainAttachDevice = libxlDomainAttachDevice, /* 0.9.2 */ + .domainAttachDeviceFlags = libxlDomainAttachDeviceFlags, /* 0.9.2 */ + .domainDetachDevice = libxlDomainDetachDevice, /* 0.9.2 */ + .domainDetachDeviceFlags = libxlDomainDetachDeviceFlags, /* 0.9.2 */ + .domainUpdateDeviceFlags = libxlDomainUpdateDeviceFlags, /* 0.9.2 */ .domainGetAutostart = libxlDomainGetAutostart, /* 0.9.0 */ .domainSetAutostart = libxlDomainSetAutostart, /* 0.9.0 */ .domainGetSchedulerType = libxlDomainGetSchedulerType, /* 0.9.0 */ -- 1.7.5.1

Am Mittwoch 18 Mai 2011 10:06:38 schrieb Markus Groß:
Based on the device attach/detach code from the QEMU driver. --- src/libxl/libxl_driver.c | 519 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 519 insertions(+), 0 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a14ace1..a056be9 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c
+ + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { + /* If dev exists it was created to modify the domain config. Free it, */
s/,/./
+ virDomainDeviceDefFree(dev); + if (!(dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + switch (action) { + case LIBXL_DEVICE_ATTACH: + ret = libxlDomainAttachDeviceLive(priv, vm, dev); + break; + case LIBXL_DEVICE_DETACH: + ret = libxlDomainDetachDeviceLive(priv, vm, dev); + break; + case LIBXL_DEVICE_UPDATE: + ret = libxlDomainUpdateDeviceLive(priv, vm, dev); + default: + libxlError(VIR_ERR_INTERNAL_ERROR, + _("unknown domain modify action %d"), action); + break; + }

Markus Groß wrote:
Based on the device attach/detach code from the QEMU driver. --- src/libxl/libxl_driver.c | 519 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 519 insertions(+), 0 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a14ace1..a056be9 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2219,6 +2219,520 @@ libxlDomainUndefine(virDomainPtr dom) return ret; }
+static int +libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, virDomainDiskDefPtr disk) +{ + virDomainDiskDefPtr origdisk = NULL; + libxl_device_disk x_disk; + int i; + int ret = -1; + + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (vm->def->disks[i]->bus == disk->bus && + STREQ(vm->def->disks[i]->dst, disk->dst)) { + origdisk = vm->def->disks[i]; + break; + } + } + + if (!origdisk) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("No device with bus '%s' and target '%s'"), + virDomainDiskBusTypeToString(disk->bus), disk->dst); + goto cleanup; + } + + if (origdisk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("Removable media not supported for %s device"), + virDomainDiskDeviceTypeToString(disk->device)); + return -1; + } + + if (libxlMakeDisk(disk, &x_disk) < 0) + goto cleanup; + + if ((ret = libxl_cdrom_insert(&priv->ctx, vm->def->id, &x_disk)) < 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight failed to change media for disk '%s'"), + disk->dst); + goto cleanup; + } + + VIR_FREE(origdisk->src); + origdisk->src = disk->src; + disk->src = NULL; + origdisk->type = disk->type; + + + virDomainDiskDefFree(disk); + + ret = 0; + +cleanup: + return ret; +} + +static int +libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr l_disk = dev->data.disk; + libxl_device_disk x_disk; + int ret = -1; + + switch (l_disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + ret = libxlDomainChangeEjectableMedia(priv, vm, l_disk); + break; + case VIR_DOMAIN_DISK_DEVICE_DISK: + if (l_disk->bus == VIR_DOMAIN_DISK_BUS_XEN) { + if (virDomainDiskIndexByName(vm->def, l_disk->dst) >= 0) { + libxlError(VIR_ERR_OPERATION_FAILED, + _("target %s already exists"), l_disk->dst); + goto cleanup; + } + + if (!l_disk->src) { + libxlError(VIR_ERR_INTERNAL_ERROR, + "%s", _("disk source path is missing")); + goto cleanup; + } + + if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (libxlMakeDisk(l_disk, &x_disk) < 0) + goto cleanup;
The domid field of libxl_device_disk struct needs to be populated. Without it, the device is not removed - all the xenstore entries and both front and back-ends still exist. I set 'x_disk.domid = vm->def->id' here in gdb and everything seemed fine, but frontend did not cleanup entirely - I could still see the device in the domain. I suspect this is a problem in the libxl, but it will have to wait for more debugging. I'm calling it a day. Do you have time for a proper patch to populate domid? Probably push that to libxlMakeDisk(). Thanks Markus, Jim
+ + if ((ret = libxl_device_disk_add(&priv->ctx, vm->def->id, + &x_disk)) < 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight failed to attach disk '%s'"), + l_disk->dst); + goto cleanup; + } + + virDomainDiskInsertPreAlloced(vm->def, l_disk); + + } else { + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be hotplugged."), + virDomainDiskBusTypeToString(l_disk->bus)); + } + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk device type '%s' cannot be hotplugged"), + virDomainDiskDeviceTypeToString(l_disk->device)); + break; + } + +cleanup: + return ret; +} + +static int +libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr l_disk = NULL; + libxl_device_disk x_disk; + int i; + int wait_secs = 2; + int ret = -1; + + switch (dev->data.disk->device) { + case VIR_DOMAIN_DISK_DEVICE_DISK: + if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_XEN) { + + if ((i = virDomainDiskIndexByName(vm->def, + dev->data.disk->dst)) < 0) { + libxlError(VIR_ERR_OPERATION_FAILED, + _("disk %s not found"), dev->data.disk->dst); + goto cleanup; + } + + l_disk = vm->def->disks[i]; + + if (libxlMakeDisk(l_disk, &x_disk) < 0) + goto cleanup; + + if ((ret = libxl_device_disk_del(&priv->ctx, &x_disk, + wait_secs)) < 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight failed to detach disk '%s'"), + l_disk->dst); + goto cleanup; + } + + virDomainDiskRemove(vm->def, i); + virDomainDiskDefFree(l_disk); + + } else { + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be hot unplugged."), + virDomainDiskBusTypeToString(l_disk->bus)); + } + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot hot unplugged"), + virDomainDiskDeviceTypeToString(dev->data.disk->device)); + break; + } + +cleanup: + return ret; +} + +static int +libxlDomainAttachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + ret = libxlDomainAttachDeviceDiskLive(priv, vm, dev); + if (!ret) + dev->data.disk = NULL; + break; + + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot be attached"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} + +static int +libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (virDomainDiskIndexByName(vmdef, disk->dst) >= 0) { + libxlError(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; + break; + + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("persistent attach of device is not supported")); + return -1; + } + return 0; +} + +static int +libxlDomainDetachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + ret = libxlDomainDetachDeviceDiskLive(priv, vm, dev); + break; + + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot be detached"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} + +static int +libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (virDomainDiskRemoveByName(vmdef, disk->dst)) { + libxlError(VIR_ERR_INVALID_ARG, + _("no target device %s"), disk->dst); + break; + } + ret = 0; + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("persistent detach of device is not supported")); + break; + } + + return ret; +} + +static int +libxlDomainUpdateDeviceLive(libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + switch (disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + ret = libxlDomainChangeEjectableMedia(priv, vm, disk); + if (ret == 0) + dev->data.disk = NULL; + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be updated."), + virDomainDiskBusTypeToString(disk->bus)); + break; + } + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot be updated"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} + +static int +libxlDomainUpdateDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr orig; + virDomainDiskDefPtr disk; + int i; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if ((i = virDomainDiskIndexByName(vmdef, disk->dst)) < 0) { + libxlError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exists."), disk->dst); + goto cleanup; + } + orig = vmdef->disks[i]; + if (!(orig->device == VIR_DOMAIN_DISK_DEVICE_CDROM)) { + libxlError(VIR_ERR_INVALID_ARG, + _("this disk doesn't support update")); + goto cleanup; + } + + VIR_FREE(orig->src); + orig->src = disk->src; + orig->type = disk->type; + 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: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("persistent update of device is not supported")); + goto cleanup; + } + + ret = 0; + +cleanup: + return ret; +} + +/* Actions for libxlDomainModifyDeviceFlags */ +enum { + LIBXL_DEVICE_ATTACH, + LIBXL_DEVICE_DETACH, + LIBXL_DEVICE_UPDATE, +}; + +static int +libxlDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags, int action) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainDefPtr vmdef = NULL; + virDomainDeviceDefPtr dev = NULL; + libxlDomainObjPrivatePtr priv; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + + libxlDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (!vm) { + libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching uuid")); + goto cleanup; + } + + 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) { + libxlError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is not running")); + goto cleanup; + } + } + + if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) { + libxlError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot modify device on transient domain")); + goto cleanup; + } + + if (!(dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + priv = vm->privateData; + + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { + if (!(dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + /* Make a copy for updated domain. */ + if (!(vmdef = virDomainObjCopyPersistentDef(driver->caps, vm))) + goto cleanup; + + switch (action) { + case LIBXL_DEVICE_ATTACH: + ret = libxlDomainAttachDeviceConfig(vmdef, dev); + break; + case LIBXL_DEVICE_DETACH: + ret = libxlDomainDetachDeviceConfig(vmdef, dev); + break; + case LIBXL_DEVICE_UPDATE: + ret = libxlDomainUpdateDeviceConfig(vmdef, dev); + default: + libxlError(VIR_ERR_INTERNAL_ERROR, + _("unknown domain modify action %d"), action); + break; + } + } else + ret = 0; + + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { + /* If dev exists it was created to modify the domain config. Free it, */ + virDomainDeviceDefFree(dev); + if (!(dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + switch (action) { + case LIBXL_DEVICE_ATTACH: + ret = libxlDomainAttachDeviceLive(priv, vm, dev); + break; + case LIBXL_DEVICE_DETACH: + ret = libxlDomainDetachDeviceLive(priv, vm, dev); + break; + case LIBXL_DEVICE_UPDATE: + ret = libxlDomainUpdateDeviceLive(priv, vm, dev); + default: + libxlError(VIR_ERR_INTERNAL_ERROR, + _("unknown domain modify action %d"), action); + break; + } + /* + * update domain status forcibly because the domain status may be + * changed even if we attach the device failed. + */ + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) + ret = -1; + } + + /* Finally, if no error until here, we can save config. */ + if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) { + ret = virDomainSaveConfig(driver->configDir, vmdef); + if (!ret) { + virDomainObjAssignDef(vm, vmdef, false); + vmdef = NULL; + } + } + +cleanup: + virDomainDefFree(vmdef); + virDomainDeviceDefFree(dev); + if (vm) + virDomainObjUnlock(vm); + libxlDriverUnlock(driver); + return ret; +} + +static int +libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) +{ + return libxlDomainModifyDeviceFlags(dom, xml, flags, LIBXL_DEVICE_ATTACH); +} + +static int +libxlDomainAttachDevice(virDomainPtr dom, const char *xml) +{ + return libxlDomainAttachDeviceFlags(dom, xml, + VIR_DOMAIN_DEVICE_MODIFY_LIVE); +} + +static int +libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) +{ + return libxlDomainModifyDeviceFlags(dom, xml, flags, LIBXL_DEVICE_DETACH); +} + +static int +libxlDomainDetachDevice(virDomainPtr dom, const char *xml) +{ + return libxlDomainDetachDeviceFlags(dom, xml, + VIR_DOMAIN_DEVICE_MODIFY_LIVE); +} + +static int +libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) +{ + return libxlDomainModifyDeviceFlags(dom, xml, flags, LIBXL_DEVICE_UPDATE); +} + static unsigned long long libxlNodeGetFreeMemory(virConnectPtr conn) { @@ -2736,6 +3250,11 @@ static virDriver libxlDriver = { .domainCreateWithFlags = libxlDomainCreateWithFlags, /* 0.9.0 */ .domainDefineXML = libxlDomainDefineXML, /* 0.9.0 */ .domainUndefine = libxlDomainUndefine, /* 0.9.0 */ + .domainAttachDevice = libxlDomainAttachDevice, /* 0.9.2 */ + .domainAttachDeviceFlags = libxlDomainAttachDeviceFlags, /* 0.9.2 */ + .domainDetachDevice = libxlDomainDetachDevice, /* 0.9.2 */ + .domainDetachDeviceFlags = libxlDomainDetachDeviceFlags, /* 0.9.2 */ + .domainUpdateDeviceFlags = libxlDomainUpdateDeviceFlags, /* 0.9.2 */ .domainGetAutostart = libxlDomainGetAutostart, /* 0.9.0 */ .domainSetAutostart = libxlDomainSetAutostart, /* 0.9.0 */ .domainGetSchedulerType = libxlDomainGetSchedulerType, /* 0.9.0 */

Quoting Jim Fehlig <jfehlig@novell.com>:
Markus Groß wrote:
Based on the device attach/detach code from the QEMU driver. --- src/libxl/libxl_driver.c | 519 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 519 insertions(+), 0 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a14ace1..a056be9 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2219,6 +2219,520 @@ libxlDomainUndefine(virDomainPtr dom) return ret; }
+static int +libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, virDomainDiskDefPtr disk) +{ + virDomainDiskDefPtr origdisk = NULL; + libxl_device_disk x_disk; + int i; + int ret = -1; + + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (vm->def->disks[i]->bus == disk->bus && + STREQ(vm->def->disks[i]->dst, disk->dst)) { + origdisk = vm->def->disks[i]; + break; + } + } + + if (!origdisk) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("No device with bus '%s' and target '%s'"), + virDomainDiskBusTypeToString(disk->bus), disk->dst); + goto cleanup; + } + + if (origdisk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("Removable media not supported for %s device"), + virDomainDiskDeviceTypeToString(disk->device)); + return -1; + } + + if (libxlMakeDisk(disk, &x_disk) < 0) + goto cleanup; + + if ((ret = libxl_cdrom_insert(&priv->ctx, vm->def->id, &x_disk)) < 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight failed to change media for disk '%s'"), + disk->dst); + goto cleanup; + } + + VIR_FREE(origdisk->src); + origdisk->src = disk->src; + disk->src = NULL; + origdisk->type = disk->type; + + + virDomainDiskDefFree(disk); + + ret = 0; + +cleanup: + return ret; +} + +static int +libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr l_disk = dev->data.disk; + libxl_device_disk x_disk; + int ret = -1; + + switch (l_disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + ret = libxlDomainChangeEjectableMedia(priv, vm, l_disk); + break; + case VIR_DOMAIN_DISK_DEVICE_DISK: + if (l_disk->bus == VIR_DOMAIN_DISK_BUS_XEN) { + if (virDomainDiskIndexByName(vm->def, l_disk->dst) >= 0) { + libxlError(VIR_ERR_OPERATION_FAILED, + _("target %s already exists"), l_disk->dst); + goto cleanup; + } + + if (!l_disk->src) { + libxlError(VIR_ERR_INTERNAL_ERROR, + "%s", _("disk source path is missing")); + goto cleanup; + } + + if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (libxlMakeDisk(l_disk, &x_disk) < 0) + goto cleanup;
The domid field of libxl_device_disk struct needs to be populated. Without it, the device is not removed - all the xenstore entries and both front and back-ends still exist. I set 'x_disk.domid = vm->def->id' here in gdb and everything seemed fine, but frontend did not cleanup entirely - I could still see the device in the domain. I suspect this is a problem in the libxl, but it will have to wait for more debugging. I'm calling it a day.
Do you have time for a proper patch to populate domid? Probably push that to libxlMakeDisk().
Ah good catch. In my tests I had to restart the domain to see the change. I will post a reworked version on monday together with the rest of the libxl patches. Thanks for the review. Cheers, Markus
Thanks Markus, Jim
+ + if ((ret = libxl_device_disk_add(&priv->ctx, vm->def->id, + &x_disk)) < 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight failed to attach disk '%s'"), + l_disk->dst); + goto cleanup; + } + + virDomainDiskInsertPreAlloced(vm->def, l_disk); + + } else { + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be hotplugged."), + virDomainDiskBusTypeToString(l_disk->bus)); + } + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk device type '%s' cannot be hotplugged"), + virDomainDiskDeviceTypeToString(l_disk->device)); + break; + } + +cleanup: + return ret; +} + +static int +libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr l_disk = NULL; + libxl_device_disk x_disk; + int i; + int wait_secs = 2; + int ret = -1; + + switch (dev->data.disk->device) { + case VIR_DOMAIN_DISK_DEVICE_DISK: + if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_XEN) { + + if ((i = virDomainDiskIndexByName(vm->def, + dev->data.disk->dst)) < 0) { + libxlError(VIR_ERR_OPERATION_FAILED, + _("disk %s not found"), dev->data.disk->dst); + goto cleanup; + } + + l_disk = vm->def->disks[i]; + + if (libxlMakeDisk(l_disk, &x_disk) < 0) + goto cleanup; + + if ((ret = libxl_device_disk_del(&priv->ctx, &x_disk, + wait_secs)) < 0) { + libxlError(VIR_ERR_INTERNAL_ERROR, + _("libxenlight failed to detach disk '%s'"), + l_disk->dst); + goto cleanup; + } + + virDomainDiskRemove(vm->def, i); + virDomainDiskDefFree(l_disk); + + } else { + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be hot unplugged."), + virDomainDiskBusTypeToString(l_disk->bus)); + } + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot hot unplugged"), + virDomainDiskDeviceTypeToString(dev->data.disk->device)); + break; + } + +cleanup: + return ret; +} + +static int +libxlDomainAttachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + ret = libxlDomainAttachDeviceDiskLive(priv, vm, dev); + if (!ret) + dev->data.disk = NULL; + break; + + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot be attached"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} + +static int +libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (virDomainDiskIndexByName(vmdef, disk->dst) >= 0) { + libxlError(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; + break; + + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("persistent attach of device is not supported")); + return -1; + } + return 0; +} + +static int +libxlDomainDetachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + ret = libxlDomainDetachDeviceDiskLive(priv, vm, dev); + break; + + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot be detached"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} + +static int +libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if (virDomainDiskRemoveByName(vmdef, disk->dst)) { + libxlError(VIR_ERR_INVALID_ARG, + _("no target device %s"), disk->dst); + break; + } + ret = 0; + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("persistent detach of device is not supported")); + break; + } + + return ret; +} + +static int +libxlDomainUpdateDeviceLive(libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr disk; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + switch (disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + ret = libxlDomainChangeEjectableMedia(priv, vm, disk); + if (ret == 0) + dev->data.disk = NULL; + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk bus '%s' cannot be updated."), + virDomainDiskBusTypeToString(disk->bus)); + break; + } + break; + default: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, + _("device type '%s' cannot be updated"), + virDomainDeviceTypeToString(dev->type)); + break; + } + + return ret; +} + +static int +libxlDomainUpdateDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) +{ + virDomainDiskDefPtr orig; + virDomainDiskDefPtr disk; + int i; + int ret = -1; + + switch (dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + disk = dev->data.disk; + if ((i = virDomainDiskIndexByName(vmdef, disk->dst)) < 0) { + libxlError(VIR_ERR_INVALID_ARG, + _("target %s doesn't exists."), disk->dst); + goto cleanup; + } + orig = vmdef->disks[i]; + if (!(orig->device == VIR_DOMAIN_DISK_DEVICE_CDROM)) { + libxlError(VIR_ERR_INVALID_ARG, + _("this disk doesn't support update")); + goto cleanup; + } + + VIR_FREE(orig->src); + orig->src = disk->src; + orig->type = disk->type; + 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: + libxlError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("persistent update of device is not supported")); + goto cleanup; + } + + ret = 0; + +cleanup: + return ret; +} + +/* Actions for libxlDomainModifyDeviceFlags */ +enum { + LIBXL_DEVICE_ATTACH, + LIBXL_DEVICE_DETACH, + LIBXL_DEVICE_UPDATE, +}; + +static int +libxlDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags, int action) +{ + libxlDriverPrivatePtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + virDomainDefPtr vmdef = NULL; + virDomainDeviceDefPtr dev = NULL; + libxlDomainObjPrivatePtr priv; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | + VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); + + libxlDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (!vm) { + libxlError(VIR_ERR_NO_DOMAIN, "%s", _("no domain with matching uuid")); + goto cleanup; + } + + 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) { + libxlError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is not running")); + goto cleanup; + } + } + + if ((flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) && !vm->persistent) { + libxlError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot modify device on transient domain")); + goto cleanup; + } + + if (!(dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + priv = vm->privateData; + + if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { + if (!(dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + /* Make a copy for updated domain. */ + if (!(vmdef = virDomainObjCopyPersistentDef(driver->caps, vm))) + goto cleanup; + + switch (action) { + case LIBXL_DEVICE_ATTACH: + ret = libxlDomainAttachDeviceConfig(vmdef, dev); + break; + case LIBXL_DEVICE_DETACH: + ret = libxlDomainDetachDeviceConfig(vmdef, dev); + break; + case LIBXL_DEVICE_UPDATE: + ret = libxlDomainUpdateDeviceConfig(vmdef, dev); + default: + libxlError(VIR_ERR_INTERNAL_ERROR, + _("unknown domain modify action %d"), action); + break; + } + } else + ret = 0; + + if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { + /* If dev exists it was created to modify the domain config. Free it, */ + virDomainDeviceDefFree(dev); + if (!(dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + switch (action) { + case LIBXL_DEVICE_ATTACH: + ret = libxlDomainAttachDeviceLive(priv, vm, dev); + break; + case LIBXL_DEVICE_DETACH: + ret = libxlDomainDetachDeviceLive(priv, vm, dev); + break; + case LIBXL_DEVICE_UPDATE: + ret = libxlDomainUpdateDeviceLive(priv, vm, dev); + default: + libxlError(VIR_ERR_INTERNAL_ERROR, + _("unknown domain modify action %d"), action); + break; + } + /* + * update domain status forcibly because the domain status may be + * changed even if we attach the device failed. + */ + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) + ret = -1; + } + + /* Finally, if no error until here, we can save config. */ + if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) { + ret = virDomainSaveConfig(driver->configDir, vmdef); + if (!ret) { + virDomainObjAssignDef(vm, vmdef, false); + vmdef = NULL; + } + } + +cleanup: + virDomainDefFree(vmdef); + virDomainDeviceDefFree(dev); + if (vm) + virDomainObjUnlock(vm); + libxlDriverUnlock(driver); + return ret; +} + +static int +libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) +{ + return libxlDomainModifyDeviceFlags(dom, xml, flags, LIBXL_DEVICE_ATTACH); +} + +static int +libxlDomainAttachDevice(virDomainPtr dom, const char *xml) +{ + return libxlDomainAttachDeviceFlags(dom, xml, + VIR_DOMAIN_DEVICE_MODIFY_LIVE); +} + +static int +libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) +{ + return libxlDomainModifyDeviceFlags(dom, xml, flags, LIBXL_DEVICE_DETACH); +} + +static int +libxlDomainDetachDevice(virDomainPtr dom, const char *xml) +{ + return libxlDomainDetachDeviceFlags(dom, xml, + VIR_DOMAIN_DEVICE_MODIFY_LIVE); +} + +static int +libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, + unsigned int flags) +{ + return libxlDomainModifyDeviceFlags(dom, xml, flags, LIBXL_DEVICE_UPDATE); +} + static unsigned long long libxlNodeGetFreeMemory(virConnectPtr conn) { @@ -2736,6 +3250,11 @@ static virDriver libxlDriver = { .domainCreateWithFlags = libxlDomainCreateWithFlags, /* 0.9.0 */ .domainDefineXML = libxlDomainDefineXML, /* 0.9.0 */ .domainUndefine = libxlDomainUndefine, /* 0.9.0 */ + .domainAttachDevice = libxlDomainAttachDevice, /* 0.9.2 */ + .domainAttachDeviceFlags = libxlDomainAttachDeviceFlags, /* 0.9.2 */ + .domainDetachDevice = libxlDomainDetachDevice, /* 0.9.2 */ + .domainDetachDeviceFlags = libxlDomainDetachDeviceFlags, /* 0.9.2 */ + .domainUpdateDeviceFlags = libxlDomainUpdateDeviceFlags, /* 0.9.2 */ .domainGetAutostart = libxlDomainGetAutostart, /* 0.9.0 */ .domainSetAutostart = libxlDomainSetAutostart, /* 0.9.0 */ .domainGetSchedulerType = libxlDomainGetSchedulerType, /* 0.9.0 */

On Wed, May 18, 2011 at 10:06:35AM +0200, Markus Groß wrote:
This patch series adds the infrastructure to support attaching and detaching of devices to the libxl driver. Currently only disk devices are supported.
The first patch refactors some functions that are needed to create the appropriate datastructures for the libxenlight interface. The second patch fixes the handling of vm def's on cleanup. The last patch finally adds the device attach/detach code and follows qemu's implementation.
Markus Groß (3): Refactored libxl datastructure instantiation Fix libxl vm def handling on domU cleanup Add disk attach/detach support to libxl driver
src/libxl/libxl_conf.c | 304 ++++++++++++++------------- src/libxl/libxl_conf.h | 8 + src/libxl/libxl_driver.c | 526 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 697 insertions(+), 141 deletions(-)
Okay, the patch set looks fine. Somehow 2/3 is independant since it's a bug fix on the current state but I kept the patch order. I also applied the small comment fix on 3/3, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel Veillard
-
Jim Fehlig
-
Markus Groß