The hotplug code in QEMU was leaking memory because although the
inner device object was being moved into the main virDomainDefPtr
config object, the outer container virDomainDeviceDefPtr was not.
* src/qemu/qemu_driver.c: Clarify code to show that the inner
device object is owned by the main domain config upon
successfull attach.
---
src/qemu/qemu_driver.c | 190 ++++++++++++++++++++++++++----------------------
1 files changed, 103 insertions(+), 87 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5054bcf..cb6fe86 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5088,17 +5088,16 @@ cleanup:
static int qemudDomainChangeEjectableMedia(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
+ virDomainDiskDefPtr disk)
{
- virDomainDiskDefPtr origdisk = NULL, newdisk;
+ virDomainDiskDefPtr origdisk = NULL;
int i;
int ret;
origdisk = NULL;
- newdisk = dev->data.disk;
for (i = 0 ; i < vm->def->ndisks ; i++) {
- if (vm->def->disks[i]->bus == newdisk->bus &&
- STREQ(vm->def->disks[i]->dst, newdisk->dst)) {
+ if (vm->def->disks[i]->bus == disk->bus &&
+ STREQ(vm->def->disks[i]->dst, disk->dst)) {
origdisk = vm->def->disks[i];
break;
}
@@ -5107,8 +5106,8 @@ static int qemudDomainChangeEjectableMedia(virConnectPtr conn,
if (!origdisk) {
qemudReportError(conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
_("No device with bus '%s' and target '%s'"),
- virDomainDiskBusTypeToString(newdisk->bus),
- newdisk->dst);
+ virDomainDiskBusTypeToString(disk->bus),
+ disk->dst);
return -1;
}
@@ -5122,28 +5121,28 @@ static int qemudDomainChangeEjectableMedia(virConnectPtr conn,
origdisk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) {
qemudReportError(conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
_("Removable media not supported for %s device"),
- virDomainDiskDeviceTypeToString(newdisk->device));
+ virDomainDiskDeviceTypeToString(disk->device));
return -1;
}
if (driver->securityDriver &&
driver->securityDriver->domainSetSecurityImageLabel &&
- driver->securityDriver->domainSetSecurityImageLabel(conn, vm, newdisk) < 0)
+ driver->securityDriver->domainSetSecurityImageLabel(conn, vm, disk) < 0)
return -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- if (newdisk->src) {
+ if (disk->src) {
const char *format = NULL;
- if (newdisk->type != VIR_DOMAIN_DISK_TYPE_DIR) {
- if (newdisk->driverType)
- format = newdisk->driverType;
+ if (disk->type != VIR_DOMAIN_DISK_TYPE_DIR) {
+ if (disk->driverType)
+ format = disk->driverType;
else if (origdisk->driverType)
format = origdisk->driverType;
}
ret = qemuMonitorChangeMedia(priv->mon,
origdisk->info.alias,
- newdisk->src, format);
+ disk->src, format);
} else {
ret = qemuMonitorEjectMedia(priv->mon, origdisk->info.alias);
}
@@ -5158,17 +5157,19 @@ static int qemudDomainChangeEjectableMedia(virConnectPtr conn,
VIR_WARN("Unable to restore security label on ejected image %s", origdisk->src);
VIR_FREE(origdisk->src);
- origdisk->src = newdisk->src;
- newdisk->src = NULL;
- origdisk->type = newdisk->type;
+ origdisk->src = disk->src;
+ disk->src = NULL;
+ origdisk->type = disk->type;
+
+ virDomainDiskDefFree(disk);
return ret;
error:
if (driver->securityDriver &&
driver->securityDriver->domainRestoreSecurityImageLabel &&
- driver->securityDriver->domainRestoreSecurityImageLabel(conn, vm, newdisk) < 0)
- VIR_WARN("Unable to restore security label on new media %s", newdisk->src);
+ driver->securityDriver->domainRestoreSecurityImageLabel(conn, vm, disk) < 0)
+ VIR_WARN("Unable to restore security label on new media %s", disk->src);
return -1;
}
@@ -5176,24 +5177,24 @@ error:
static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
+ virDomainDiskDefPtr disk)
{
int i, ret;
- const char* type = virDomainDiskBusTypeToString(dev->data.disk->bus);
+ const char* type = virDomainDiskBusTypeToString(disk->bus);
qemuDomainObjPrivatePtr priv = vm->privateData;
virDomainDevicePCIAddress guestAddr;
for (i = 0 ; i < vm->def->ndisks ; i++) {
- if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) {
+ if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _("target %s already exists"), dev->data.disk->dst);
+ _("target %s already exists"), disk->dst);
return -1;
}
}
if (driver->securityDriver &&
driver->securityDriver->domainSetSecurityImageLabel &&
- driver->securityDriver->domainSetSecurityImageLabel(conn, vm, dev->data.disk) < 0)
+ driver->securityDriver->domainSetSecurityImageLabel(conn, vm, disk) < 0)
return -1;
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) {
@@ -5203,7 +5204,7 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
ret = qemuMonitorAddPCIDisk(priv->mon,
- dev->data.disk->src,
+ disk->src,
type,
&guestAddr);
qemuDomainObjExitMonitorWithDriver(driver, vm);
@@ -5211,36 +5212,37 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
if (ret < 0)
goto error;
- dev->data.disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
- memcpy(&dev->data.disk->info.addr.pci, &guestAddr, sizeof(guestAddr));
- virDomainDiskInsertPreAlloced(vm->def, dev->data.disk);
+ disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr));
+ virDomainDiskInsertPreAlloced(vm->def, disk);
return 0;
error:
if (driver->securityDriver &&
driver->securityDriver->domainRestoreSecurityImageLabel &&
- driver->securityDriver->domainRestoreSecurityImageLabel(conn, vm, dev->data.disk) < 0)
- VIR_WARN("Unable to restore security label on %s", dev->data.disk->src);
+ driver->securityDriver->domainRestoreSecurityImageLabel(conn, vm, disk) < 0)
+ VIR_WARN("Unable to restore security label on %s", disk->src);
return -1;
}
+
static int qemudDomainAttachPciControllerDevice(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
- virDomainControllerDefPtr def)
+ virDomainControllerDefPtr controller)
{
int i, ret;
- const char* type = virDomainControllerTypeToString(def->type);
+ const char* type = virDomainControllerTypeToString(controller->type);
qemuDomainObjPrivatePtr priv = vm->privateData;
for (i = 0 ; i < vm->def->ncontrollers ; i++) {
- if ((vm->def->controllers[i]->type == def->type) &&
- (vm->def->controllers[i]->idx == def->idx)) {
+ if ((vm->def->controllers[i]->type == controller->type) &&
+ (vm->def->controllers[i]->idx == controller->idx)) {
qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
_("target %s:%d already exists"),
- type, def->idx);
+ type, controller->idx);
return -1;
}
}
@@ -5253,17 +5255,18 @@ static int qemudDomainAttachPciControllerDevice(virConnectPtr conn,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
ret = qemuMonitorAttachPCIDiskController(priv->mon,
type,
- &def->info.addr.pci);
+ &controller->info.addr.pci);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (ret == 0) {
- def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
- virDomainControllerInsertPreAlloced(vm->def, def);
+ controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ virDomainControllerInsertPreAlloced(vm->def, controller);
}
return ret;
}
+
static virDomainControllerDefPtr
qemuDomainFindOrCreateSCSIDiskController(virConnectPtr conn,
struct qemud_driver *driver,
@@ -5300,10 +5303,11 @@ qemuDomainFindOrCreateSCSIDiskController(virConnectPtr conn,
return cont;
}
+
static int qemudDomainAttachSCSIDisk(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
- virDomainDiskDefPtr dev,
+ virDomainDiskDefPtr disk,
int qemuCmdFlags)
{
int i;
@@ -5314,9 +5318,9 @@ static int qemudDomainAttachSCSIDisk(virConnectPtr conn,
int ret = -1;
for (i = 0 ; i < vm->def->ndisks ; i++) {
- if (STREQ(vm->def->disks[i]->dst, dev->dst)) {
+ if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _("target %s already exists"), dev->dst);
+ _("target %s already exists"), disk->dst);
return -1;
}
}
@@ -5324,25 +5328,25 @@ static int qemudDomainAttachSCSIDisk(virConnectPtr conn,
if (driver->securityDriver &&
driver->securityDriver->domainSetSecurityImageLabel &&
- driver->securityDriver->domainSetSecurityImageLabel(conn, vm, dev) < 0)
+ driver->securityDriver->domainSetSecurityImageLabel(conn, vm, disk) < 0)
return -1;
/* This func allocates the bus/unit IDs so must be before
* we search for controller
*/
- if (!(drivestr = qemuBuildDriveStr(dev, 0, qemuCmdFlags)))
+ if (!(drivestr = qemuBuildDriveStr(disk, 0, qemuCmdFlags)))
goto error;
/* We should have an adddress now, so make sure */
- if (dev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
+ if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("unexpected disk address type %s"),
- virDomainDeviceAddressTypeToString(dev->info.type));
+ virDomainDeviceAddressTypeToString(disk->info.type));
goto error;
}
- for (i = 0 ; i < dev->info.addr.drive.controller ; i++) {
+ for (i = 0 ; i < disk->info.addr.drive.controller ; i++) {
cont = qemuDomainFindOrCreateSCSIDiskController(conn, driver, vm, i);
if (!cont)
goto error;
@@ -5371,9 +5375,9 @@ static int qemudDomainAttachSCSIDisk(virConnectPtr conn,
/* XXX we should probably validate that the addr matches
* our existing defined addr instead of overwriting */
- dev->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
- memcpy(&dev->info.addr.drive, &driveAddr, sizeof(driveAddr));
- virDomainDiskInsertPreAlloced(vm->def, dev);
+ disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
+ memcpy(&disk->info.addr.drive, &driveAddr, sizeof(driveAddr));
+ virDomainDiskInsertPreAlloced(vm->def, disk);
VIR_FREE(drivestr);
return 0;
@@ -5382,8 +5386,8 @@ error:
VIR_FREE(drivestr);
if (driver->securityDriver &&
driver->securityDriver->domainRestoreSecurityImageLabel &&
- driver->securityDriver->domainRestoreSecurityImageLabel(conn, vm, dev) < 0)
- VIR_WARN("Unable to restore security label on %s", dev->src);
+ driver->securityDriver->domainRestoreSecurityImageLabel(conn, vm, disk) < 0)
+ VIR_WARN("Unable to restore security label on %s", disk->src);
return -1;
}
@@ -5392,25 +5396,25 @@ error:
static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
+ virDomainDiskDefPtr disk)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
int i, ret;
for (i = 0 ; i < vm->def->ndisks ; i++) {
- if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) {
+ if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
- _("target %s already exists"), dev->data.disk->dst);
+ _("target %s already exists"), disk->dst);
return -1;
}
}
if (driver->securityDriver &&
driver->securityDriver->domainSetSecurityImageLabel &&
- driver->securityDriver->domainSetSecurityImageLabel(conn, vm, dev->data.disk) < 0)
+ driver->securityDriver->domainSetSecurityImageLabel(conn, vm, disk) < 0)
return -1;
- if (!dev->data.disk->src) {
+ if (!disk->src) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("disk source path is missing"));
goto error;
@@ -5422,21 +5426,21 @@ static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn,
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- ret = qemuMonitorAddUSBDisk(priv->mon, dev->data.disk->src);
+ ret = qemuMonitorAddUSBDisk(priv->mon, disk->src);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (ret < 0)
goto error;
- virDomainDiskInsertPreAlloced(vm->def, dev->data.disk);
+ virDomainDiskInsertPreAlloced(vm->def, disk);
return 0;
error:
if (driver->securityDriver &&
driver->securityDriver->domainRestoreSecurityImageLabel &&
- driver->securityDriver->domainRestoreSecurityImageLabel(conn, vm, dev->data.disk) < 0)
- VIR_WARN("Unable to restore security label on %s", dev->data.disk->src);
+ driver->securityDriver->domainRestoreSecurityImageLabel(conn, vm, disk) < 0)
+ VIR_WARN("Unable to restore security label on %s", disk->src);
return -1;
}
@@ -5445,10 +5449,9 @@ error:
static int qemudDomainAttachNetDevice(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
- virDomainDeviceDefPtr dev,
+ virDomainNetDefPtr net,
unsigned int qemuCmdFlags)
{
- virDomainNetDefPtr net = dev->data.net;
qemuDomainObjPrivatePtr priv = vm->privateData;
char *tapfd_name = NULL;
int i, tapfd = -1;
@@ -5572,13 +5575,13 @@ no_memory:
goto cleanup;
}
+
static int qemudDomainAttachHostPciDevice(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
+ virDomainHostdevDefPtr hostdev)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- virDomainHostdevDefPtr hostdev = dev->data.hostdev;
pciDevice *pci;
int ret;
virDomainDevicePCIAddress guestAddr;
@@ -5627,10 +5630,11 @@ error:
return -1;
}
+
static int qemudDomainAttachHostUsbDevice(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
+ virDomainHostdevDefPtr hostdev)
{
int ret;
qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -5641,30 +5645,29 @@ static int qemudDomainAttachHostUsbDevice(virConnectPtr conn,
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
- if (dev->data.hostdev->source.subsys.u.usb.vendor) {
+ if (hostdev->source.subsys.u.usb.vendor) {
ret = qemuMonitorAddUSBDeviceMatch(priv->mon,
- dev->data.hostdev->source.subsys.u.usb.vendor,
- dev->data.hostdev->source.subsys.u.usb.product);
+ hostdev->source.subsys.u.usb.vendor,
+ hostdev->source.subsys.u.usb.product);
} else {
ret = qemuMonitorAddUSBDeviceExact(priv->mon,
- dev->data.hostdev->source.subsys.u.usb.bus,
- dev->data.hostdev->source.subsys.u.usb.device);
+ hostdev->source.subsys.u.usb.bus,
+ hostdev->source.subsys.u.usb.device);
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
- if (ret != -1)
- vm->def->hostdevs[vm->def->nhostdevs++] = dev->data.hostdev;
+ if (ret == 0)
+ vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
return ret;
}
+
static int qemudDomainAttachHostDevice(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
+ virDomainHostdevDefPtr hostdev)
{
- virDomainHostdevDefPtr hostdev = dev->data.hostdev;
-
if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT,
_("hostdev mode '%s' not supported"),
@@ -5674,17 +5677,17 @@ static int qemudDomainAttachHostDevice(virConnectPtr conn,
if (driver->securityDriver &&
driver->securityDriver->domainSetSecurityHostdevLabel &&
- driver->securityDriver->domainSetSecurityHostdevLabel(conn, vm, dev->data.hostdev) < 0)
+ driver->securityDriver->domainSetSecurityHostdevLabel(conn, vm, hostdev) < 0)
return -1;
switch (hostdev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
- if (qemudDomainAttachHostPciDevice(conn, driver, vm, dev) < 0)
+ if (qemudDomainAttachHostPciDevice(conn, driver, vm, hostdev) < 0)
goto error;
break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
- if (qemudDomainAttachHostUsbDevice(conn, driver, vm, dev) < 0)
+ if (qemudDomainAttachHostUsbDevice(conn, driver, vm, hostdev) < 0)
goto error;
break;
@@ -5700,14 +5703,16 @@ static int qemudDomainAttachHostDevice(virConnectPtr conn,
error:
if (driver->securityDriver &&
driver->securityDriver->domainRestoreSecurityHostdevLabel &&
- driver->securityDriver->domainRestoreSecurityHostdevLabel(conn, vm, dev->data.hostdev) < 0)
+ driver->securityDriver->domainRestoreSecurityHostdevLabel(conn, vm, hostdev) < 0)
VIR_WARN0("Unable to restore host device labelling on hotplug fail");
return -1;
}
+
static int qemudDomainAttachDevice(virDomainPtr dom,
- const char *xml) {
+ const char *xml)
+{
struct qemud_driver *driver = dom->conn->privateData;
virDomainObjPtr vm;
virDomainDeviceDefPtr dev = NULL;
@@ -5766,16 +5771,24 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
switch (dev->data.disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
- ret = qemudDomainChangeEjectableMedia(dom->conn, driver, vm, dev);
+ ret = qemudDomainChangeEjectableMedia(dom->conn, driver, vm, dev->data.disk);
+ 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 = qemudDomainAttachUsbMassstorageDevice(dom->conn, driver, vm, dev);
+ ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, driver, vm, dev->data.disk);
+ if (ret == 0)
+ dev->data.disk = NULL;
} else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
- ret = qemudDomainAttachPciDiskDevice(dom->conn, driver, vm, dev);
+ ret = qemudDomainAttachPciDiskDevice(dom->conn, driver, vm, dev->data.disk);
+ if (ret == 0)
+ dev->data.disk = NULL;
} else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
ret = qemudDomainAttachSCSIDisk(dom->conn, driver, vm, dev->data.disk, qemuCmdFlags);
+ if (ret == 0)
+ dev->data.disk = NULL;
} else {
qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
_("disk bus '%s' cannot be hotplugged."),
@@ -5804,9 +5817,13 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
/* fallthrough */
}
} else if (dev->type == VIR_DOMAIN_DEVICE_NET) {
- ret = qemudDomainAttachNetDevice(dom->conn, driver, vm, dev, qemuCmdFlags);
+ ret = qemudDomainAttachNetDevice(dom->conn, driver, vm, dev->data.net, qemuCmdFlags);
+ if (ret == 0)
+ dev->data.net = NULL;
} else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
- ret = qemudDomainAttachHostDevice(dom->conn, driver, vm, dev);
+ ret = qemudDomainAttachHostDevice(dom->conn, driver, vm, dev->data.hostdev);
+ if (ret == 0)
+ dev->data.hostdev = NULL;
} else {
qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
_("device type '%s' cannot be attached"),
@@ -5825,8 +5842,7 @@ cleanup:
if (cgroup)
virCgroupFree(&cgroup);
- if (ret < 0 && dev != NULL)
- virDomainDeviceDefFree(dev);
+ virDomainDeviceDefFree(dev);
if (vm)
virDomainObjUnlock(vm);
qemuDriverUnlock(driver);
--
1.6.5.2
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list