[libvirt] [PATCH 0/7] Update hotplug code to use device_add

I previously changed the command line syntax to use -device for starting guests. The key reason for this was to allow unique names and PCI addresses to be assigned to all devices. I forgot todo the same for hotplug :-( This series is not yet complete, but it goes some of the way towards fixing the hotplug code to use 'device_add' for creating devices. The main remaining problem is that I'm nt assigning aliases to devices, to they all get the name of '(null)'. Obviously I'll fix that before submitting again... Daniel

All the helper functions for building command line arguments now return a 'char *', instead of acepting a 'char **' or virBufferPtr argument * qemu/qemu_conf.c: Standardize syntax for building args * qemu/qemu_conf.h: Export all functions for building args * qemu/qemu_driver.c: Update for changed syntax for building NIC/hostnet args --- src/qemu/qemu_conf.c | 341 +++++++++++++++++++++++------------------------- src/qemu/qemu_conf.h | 56 ++++++-- src/qemu/qemu_driver.c | 6 +- 3 files changed, 211 insertions(+), 192 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f4a6c08..e11ec35 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2056,10 +2056,10 @@ error: return NULL; } -static int + +char * qemuBuildDriveDevStr(virConnectPtr conn, - virDomainDiskDefPtr disk, - char **str) + virDomainDiskDefPtr disk) { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); @@ -2100,17 +2100,20 @@ qemuBuildDriveDevStr(virConnectPtr conn, virBufferVSprintf(&opt, ",drive=drive-%s", disk->info.alias); virBufferVSprintf(&opt, ",id=%s", disk->info.alias); - *str = virBufferContentAndReset(&opt); - return 0; + if (virBufferError(&opt)) { + virReportOOMError(NULL); + goto error; + } + + return virBufferContentAndReset(&opt); error: virBufferFreeAndReset(&opt); - *str = NULL; - return -1; + return NULL; } -static char * +char * qemuBuildControllerDevStr(virDomainControllerDefPtr def) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2133,8 +2136,10 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def) if (qemuBuildDeviceAddressStr(&buf, &def->info) < 0) goto error; - if (virBufferError(&buf)) + if (virBufferError(&buf)) { + virReportOOMError(NULL); goto error; + } return virBufferContentAndReset(&buf); @@ -2144,14 +2149,14 @@ error: } -int +char * qemuBuildNicStr(virConnectPtr conn, virDomainNetDefPtr net, const char *prefix, - int vlan, - char **str) + int vlan) { - if (virAsprintf(str, + char *str; + if (virAsprintf(&str, "%smacaddr=%02x:%02x:%02x:%02x:%02x:%02x,vlan=%d%s%s%s%s", prefix ? prefix : "", net->mac[0], net->mac[1], @@ -2163,13 +2168,14 @@ qemuBuildNicStr(virConnectPtr conn, (net->info.alias ? ",name=" : ""), (net->info.alias ? net->info.alias : "")) < 0) { virReportOOMError(conn); - return -1; + return NULL; } - return 0; + return str; } -static char * + +char * qemuBuildNicDevStr(virDomainNetDefPtr net) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2204,23 +2210,25 @@ error: return NULL; } -int + +char * qemuBuildHostNetStr(virConnectPtr conn, virDomainNetDefPtr net, char type_sep, int vlan, - const char *tapfd, - char **str) + const char *tapfd) { + char *str = NULL; + switch (net->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_BRIDGE: - if (virAsprintf(str, "tap%cfd=%s,vlan=%d%s%s", + if (virAsprintf(&str, "tap%cfd=%s,vlan=%d%s%s", type_sep, tapfd, vlan, (net->hostnet_name ? ",name=" : ""), (net->hostnet_name ? net->hostnet_name : "")) < 0) { virReportOOMError(conn); - return -1; + return NULL; } break; @@ -2247,10 +2255,10 @@ qemuBuildHostNetStr(virConnectPtr conn, if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(conn); - return -1; + return NULL; } - *str = virBufferContentAndReset(&buf); + str = virBufferContentAndReset(&buf); } break; @@ -2272,7 +2280,7 @@ qemuBuildHostNetStr(virConnectPtr conn, break; } - if (virAsprintf(str, "socket%c%s=%s:%d,vlan=%d%s%s", + if (virAsprintf(&str, "socket%c%s=%s:%d,vlan=%d%s%s", type_sep, mode, net->data.socket.address, net->data.socket.port, @@ -2280,40 +2288,41 @@ qemuBuildHostNetStr(virConnectPtr conn, (net->hostnet_name ? ",name=" : ""), (net->hostnet_name ? net->hostnet_name : "")) < 0) { virReportOOMError(conn); - return -1; + return NULL; } } break; case VIR_DOMAIN_NET_TYPE_USER: default: - if (virAsprintf(str, "user%cvlan=%d%s%s", + if (virAsprintf(&str, "user%cvlan=%d%s%s", type_sep, vlan, (net->hostnet_name ? ",name=" : ""), (net->hostnet_name ? net->hostnet_name : "")) < 0) { virReportOOMError(conn); - return -1; + return NULL; } break; } - return 0; + return str; } -static int +char * qemuBuildNetDevStr(virConnectPtr conn, virDomainNetDefPtr net, - const char *tapfd, - char **str) + const char *tapfd) { + char *str = NULL; + switch (net->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_BRIDGE: - if (virAsprintf(str, "tap,fd=%s,id=%s", + if (virAsprintf(&str, "tap,fd=%s,id=%s", tapfd, net->hostnet_name) < 0) { virReportOOMError(conn); - return -1; + return NULL; } break; @@ -2333,10 +2342,10 @@ qemuBuildNetDevStr(virConnectPtr conn, if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(conn); - return -1; + return NULL; } - *str = virBufferContentAndReset(&buf); + str = virBufferContentAndReset(&buf); } break; @@ -2358,32 +2367,33 @@ qemuBuildNetDevStr(virConnectPtr conn, break; } - if (virAsprintf(str, "socket,%s=%s:%d,id=%s", + if (virAsprintf(&str, "socket,%s=%s:%d,id=%s", mode, net->data.socket.address, net->data.socket.port, net->hostnet_name) < 0) { virReportOOMError(conn); - return -1; + return NULL; } } break; case VIR_DOMAIN_NET_TYPE_USER: default: - if (virAsprintf(str, "user,id=%s", + if (virAsprintf(&str, "user,id=%s", net->hostnet_name) < 0) { virReportOOMError(conn); - return -1; + return NULL; } break; } - return 0; + return str; } -static char *qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev) +char * +qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2412,7 +2422,8 @@ error: } -static char *qemuBuildUSBInputDevStr(virDomainInputDefPtr dev) +char * +qemuBuildUSBInputDevStr(virDomainInputDefPtr dev) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2434,7 +2445,7 @@ error: } -static char * +char * qemuBuildSoundDevStr(virDomainSoundDefPtr sound) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2470,7 +2481,7 @@ error: } -static char * +char * qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2496,43 +2507,46 @@ error: return NULL; } + /* This function outputs a -chardev command line option which describes only the * host side of the character device */ -static void qemudBuildCommandLineChrDevChardevStr(virDomainChrDefPtr dev, - virBufferPtr buf) +char * +qemuBuildChrChardevStr(virDomainChrDefPtr dev) { + virBuffer buf = VIR_BUFFER_INITIALIZER; bool telnet; + switch(dev->type) { case VIR_DOMAIN_CHR_TYPE_NULL: - virBufferVSprintf(buf, "null,id=%s", dev->info.alias); + virBufferVSprintf(&buf, "null,id=%s", dev->info.alias); break; case VIR_DOMAIN_CHR_TYPE_VC: - virBufferVSprintf(buf, "vc,id=%s", dev->info.alias); + virBufferVSprintf(&buf, "vc,id=%s", dev->info.alias); break; case VIR_DOMAIN_CHR_TYPE_PTY: - virBufferVSprintf(buf, "pty,id=%s", dev->info.alias); + virBufferVSprintf(&buf, "pty,id=%s", dev->info.alias); break; case VIR_DOMAIN_CHR_TYPE_DEV: - virBufferVSprintf(buf, "tty,id=%s,path=%s", dev->info.alias, dev->data.file.path); + virBufferVSprintf(&buf, "tty,id=%s,path=%s", dev->info.alias, dev->data.file.path); break; case VIR_DOMAIN_CHR_TYPE_FILE: - virBufferVSprintf(buf, "file,id=%s,path=%s", dev->info.alias, dev->data.file.path); + virBufferVSprintf(&buf, "file,id=%s,path=%s", dev->info.alias, dev->data.file.path); break; case VIR_DOMAIN_CHR_TYPE_PIPE: - virBufferVSprintf(buf, "pipe,id=%s,path=%s", dev->info.alias, dev->data.file.path); + virBufferVSprintf(&buf, "pipe,id=%s,path=%s", dev->info.alias, dev->data.file.path); break; case VIR_DOMAIN_CHR_TYPE_STDIO: - virBufferVSprintf(buf, "stdio,id=%s", dev->info.alias); + virBufferVSprintf(&buf, "stdio,id=%s", dev->info.alias); break; case VIR_DOMAIN_CHR_TYPE_UDP: - virBufferVSprintf(buf, + virBufferVSprintf(&buf, "udp,id=%s,host=%s,port=%s,localaddr=%s,localport=%s", dev->info.alias, dev->data.udp.connectHost, @@ -2543,7 +2557,7 @@ static void qemudBuildCommandLineChrDevChardevStr(virDomainChrDefPtr dev, case VIR_DOMAIN_CHR_TYPE_TCP: telnet = dev->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET; - virBufferVSprintf(buf, + virBufferVSprintf(&buf, "socket,id=%s,host=%s,port=%s%s%s", dev->info.alias, dev->data.tcp.host, @@ -2553,49 +2567,66 @@ static void qemudBuildCommandLineChrDevChardevStr(virDomainChrDefPtr dev, break; case VIR_DOMAIN_CHR_TYPE_UNIX: - virBufferVSprintf(buf, + virBufferVSprintf(&buf, "socket,id=%s,path=%s%s", dev->info.alias, dev->data.nix.path, dev->data.nix.listen ? ",server,nowait" : ""); break; } + + if (virBufferError(&buf)) { + virReportOOMError(NULL); + goto error; + } + + return virBufferContentAndReset(&buf); + +error: + virBufferFreeAndReset(&buf); + return NULL; } -static void qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, - virBufferPtr buf) + +char * +qemuBuildChrArgStr(virDomainChrDefPtr dev, const char *prefix) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (prefix) + virBufferAdd(&buf, prefix, strlen(prefix)); + switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_NULL: - virBufferAddLit(buf, "null"); + virBufferAddLit(&buf, "null"); break; case VIR_DOMAIN_CHR_TYPE_VC: - virBufferAddLit(buf, "vc"); + virBufferAddLit(&buf, "vc"); break; case VIR_DOMAIN_CHR_TYPE_PTY: - virBufferAddLit(buf, "pty"); + virBufferAddLit(&buf, "pty"); break; case VIR_DOMAIN_CHR_TYPE_DEV: - virBufferStrcat(buf, dev->data.file.path, NULL); + virBufferStrcat(&buf, dev->data.file.path, NULL); break; case VIR_DOMAIN_CHR_TYPE_FILE: - virBufferVSprintf(buf, "file:%s", dev->data.file.path); + virBufferVSprintf(&buf, "file:%s", dev->data.file.path); break; case VIR_DOMAIN_CHR_TYPE_PIPE: - virBufferVSprintf(buf, "pipe:%s", dev->data.file.path); + virBufferVSprintf(&buf, "pipe:%s", dev->data.file.path); break; case VIR_DOMAIN_CHR_TYPE_STDIO: - virBufferAddLit(buf, "stdio"); + virBufferAddLit(&buf, "stdio"); break; case VIR_DOMAIN_CHR_TYPE_UDP: - virBufferVSprintf(buf, "udp:%s:%s@%s:%s", + virBufferVSprintf(&buf, "udp:%s:%s@%s:%s", dev->data.udp.connectHost, dev->data.udp.connectService, dev->data.udp.bindHost, @@ -2604,12 +2635,12 @@ static void qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, case VIR_DOMAIN_CHR_TYPE_TCP: if (dev->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET) { - virBufferVSprintf(buf, "telnet:%s:%s%s", + virBufferVSprintf(&buf, "telnet:%s:%s%s", dev->data.tcp.host, dev->data.tcp.service, dev->data.tcp.listen ? ",server,nowait" : ""); } else { - virBufferVSprintf(buf, "tcp:%s:%s%s", + virBufferVSprintf(&buf, "tcp:%s:%s%s", dev->data.tcp.host, dev->data.tcp.service, dev->data.tcp.listen ? ",server,nowait" : ""); @@ -2617,21 +2648,32 @@ static void qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, break; case VIR_DOMAIN_CHR_TYPE_UNIX: - virBufferVSprintf(buf, "unix:%s%s", + virBufferVSprintf(&buf, "unix:%s%s", dev->data.nix.path, dev->data.nix.listen ? ",server,nowait" : ""); break; } + + if (virBufferError(&buf)) { + virReportOOMError(NULL); + goto error; + } + + return virBufferContentAndReset(&buf); + +error: + virBufferFreeAndReset(&buf); + return NULL; } static int -qemudBuildCommandLineCPU(virConnectPtr conn, - const struct qemud_driver *driver, - const virDomainDefPtr def, - const char *emulator, - const struct utsname *ut, - char **opt) +qemuBuildCpuArgStr(virConnectPtr conn, + const struct qemud_driver *driver, + const virDomainDefPtr def, + const char *emulator, + const struct utsname *ut, + char **opt) { const virCPUDefPtr host = driver->caps->host.cpu; virCPUDefPtr guest = NULL; @@ -2717,9 +2759,9 @@ no_memory: } static char * -qemudBuildCommandLineSmp(virConnectPtr conn, - const virDomainDefPtr def, - int qemuCmdFlags) +qemuBuildSmpArgStr(virConnectPtr conn, + const virDomainDefPtr def, + int qemuCmdFlags) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2949,7 +2991,7 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT(def->os.machine); } - if (qemudBuildCommandLineCPU(conn, driver, def, emulator, &ut, &cpu) < 0) + if (qemuBuildCpuArgStr(conn, driver, def, emulator, &ut, &cpu) < 0) goto error; if (cpu) { @@ -2989,7 +3031,7 @@ int qemudBuildCommandLine(virConnectPtr conn, } ADD_ARG_LIT("-smp"); - if (!(smp = qemudBuildCommandLineSmp(conn, def, qemuCmdFlags))) + if (!(smp = qemuBuildSmpArgStr(conn, def, qemuCmdFlags))) goto error; ADD_ARG(smp); @@ -3033,25 +3075,14 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT("-nodefaults"); if (monitor_chr) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - + char *chrdev; /* Use -chardev if it's available */ if (qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) { - qemudBuildCommandLineChrDevChardevStr(monitor_chr, &buf); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - goto no_memory; - } ADD_ARG_LIT("-chardev"); - ADD_ARG(virBufferContentAndReset(&buf)); - - virBufferVSprintf(&buf, "chardev=%s", monitor_chr->info.alias); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - goto no_memory; - } + if (!(chrdev = qemuBuildChrChardevStr(monitor_chr))) + goto error; + ADD_ARG(chrdev); ADD_ARG_LIT("-mon"); if (monitor_json) @@ -3059,18 +3090,14 @@ int qemudBuildCommandLine(virConnectPtr conn, else ADD_ARG_LIT("chardev=monitor,mode=readline"); } else { + const char *prefix = NULL; if (monitor_json) - virBufferAddLit(&buf, "control,"); - - qemudBuildCommandLineChrDevStr(monitor_chr, &buf); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - goto no_memory; - } + prefix = "control,"; ADD_ARG_LIT("-monitor"); - ADD_ARG(virBufferContentAndReset(&buf)); + if (!(chrdev = qemuBuildChrArgStr(monitor_chr, prefix))) + goto error; + ADD_ARG(chrdev); } } @@ -3239,7 +3266,7 @@ int qemudBuildCommandLine(virConnectPtr conn, } else { ADD_ARG_LIT("-device"); - if (qemuBuildDriveDevStr(conn, disk, &optstr) < 0) + if (!(optstr = qemuBuildDriveDevStr(conn, disk))) goto error; ADD_ARG(optstr); } @@ -3346,7 +3373,7 @@ int qemudBuildCommandLine(virConnectPtr conn, if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { ADD_ARG_LIT("-netdev"); - if (qemuBuildNetDevStr(conn, net, tapfd_name, &host) < 0) + if (!(host = qemuBuildNetDevStr(conn, net, tapfd_name))) goto error; ADD_ARG(host); @@ -3356,13 +3383,13 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG(nic); } else { ADD_ARG_LIT("-net"); - if (qemuBuildNicStr(conn, net, "nic,", net->vlan, &nic) < 0) + if (!(nic = qemuBuildNicStr(conn, net, "nic,", net->vlan))) goto error; ADD_ARG(nic); ADD_ARG_LIT("-net"); - if (qemuBuildHostNetStr(conn, net, ',', - net->vlan, tapfd_name, &host) < 0) + if (!(host = qemuBuildHostNetStr(conn, net, ',', + net->vlan, tapfd_name))) goto error; ADD_ARG(host); } @@ -3377,40 +3404,26 @@ int qemudBuildCommandLine(virConnectPtr conn, } } else { for (i = 0 ; i < def->nserials ; i++) { - virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainChrDefPtr serial = def->serials[i]; + char *devstr; /* Use -chardev with -device if they are available */ if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) && (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { - qemudBuildCommandLineChrDevChardevStr(serial, &buf); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - goto no_memory; - } - ADD_ARG_LIT("-chardev"); - ADD_ARG(virBufferContentAndReset(&buf)); - - virBufferVSprintf(&buf, "isa-serial,chardev=%s", serial->info.alias); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - goto no_memory; - } + if (!(devstr = qemuBuildChrChardevStr(serial))) + goto error; + ADD_ARG(devstr); ADD_ARG_LIT("-device"); - ADD_ARG(virBufferContentAndReset(&buf)); - } - - else { - qemudBuildCommandLineChrDevStr(serial, &buf); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); + if (virAsprintf(&devstr, "isa-serial,chardev=%s", serial->info.alias) < 0) goto no_memory; - } - + ADD_ARG(devstr); + } else { ADD_ARG_LIT("-serial"); - ADD_ARG(virBufferContentAndReset(&buf)); + if (!(devstr = qemuBuildChrArgStr(serial, NULL))) + goto error; + ADD_ARG(devstr); } } } @@ -3423,47 +3436,33 @@ int qemudBuildCommandLine(virConnectPtr conn, } } else { for (i = 0 ; i < def->nparallels ; i++) { - virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainChrDefPtr parallel = def->parallels[i]; + char *devstr; /* Use -chardev with -device if they are available */ if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) && (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { - qemudBuildCommandLineChrDevChardevStr(parallel, &buf); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - goto no_memory; - } - ADD_ARG_LIT("-chardev"); - ADD_ARG(virBufferContentAndReset(&buf)); - - virBufferVSprintf(&buf, "isa-parallel,chardev=%s", parallel->info.alias); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - goto no_memory; - } + if (!(devstr = qemuBuildChrChardevStr(parallel))) + goto error; + ADD_ARG(devstr); ADD_ARG_LIT("-device"); - ADD_ARG(virBufferContentAndReset(&buf)); - } - - else { - qemudBuildCommandLineChrDevStr(parallel, &buf); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); + if (virAsprintf(&devstr, "isa-parallel,chardev=%s", parallel->info.alias) < 0) goto no_memory; - } - + ADD_ARG(devstr); + } else { ADD_ARG_LIT("-parallel"); - ADD_ARG(virBufferContentAndReset(&buf)); + if (!(devstr = qemuBuildChrArgStr(parallel, NULL))) + goto error; + ADD_ARG(devstr); } } } for (i = 0 ; i < def->nchannels ; i++) { - virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainChrDefPtr channel = def->channels[i]; + char *devstr; switch(channel->targetType) { case VIR_DOMAIN_CHR_TARGET_TYPE_GUESTFWD: @@ -3474,30 +3473,22 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - qemudBuildCommandLineChrDevChardevStr(channel, &buf); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - goto no_memory; - } - ADD_ARG_LIT("-chardev"); - ADD_ARG(virBufferContentAndReset(&buf)); + if (!(devstr = qemuBuildChrChardevStr(channel))) + goto error; + ADD_ARG(devstr); - const char *addr = virSocketFormatAddr(channel->target.addr); + char *addr = virSocketFormatAddr(channel->target.addr); int port = virSocketGetPort(channel->target.addr); ADD_ARG_LIT("-netdev"); - virBufferVSprintf(&buf, "user,guestfwd=tcp:%s:%i,chardev=%s,id=user-%s", - addr, port, channel->info.alias, channel->info.alias); - - VIR_FREE(addr); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); + if (virAsprintf(&devstr, "user,guestfwd=tcp:%s:%i,chardev=%s,id=user-%s", + addr, port, channel->info.alias, channel->info.alias) < 0) { + VIR_FREE(addr); goto no_memory; } - - ADD_ARG(virBufferContentAndReset(&buf)); + VIR_FREE(addr); + ADD_ARG(devstr); } } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 2530813..b6f128f 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -188,23 +188,51 @@ int qemudBuildCommandLine (virConnectPtr conn, int *ntapfds, const char *migrateFrom); -int qemuBuildHostNetStr (virConnectPtr conn, - virDomainNetDefPtr net, - char type_sep, - int vlan, - const char *tapfd, - char **str); +/* Legacy, pre device support */ +char * qemuBuildHostNetStr(virConnectPtr conn, + virDomainNetDefPtr net, + char type_sep, + int vlan, + const char *tapfd); -int qemuBuildNicStr (virConnectPtr conn, - virDomainNetDefPtr net, - const char *prefix, - int vlan, - char **str); +/* Current, best practice */ +char * qemuBuildNetDevStr(virConnectPtr conn, + virDomainNetDefPtr net, + const char *tapfd); + + +/* Legacy, pre device support */ +char * qemuBuildNicStr(virConnectPtr conn, + virDomainNetDefPtr net, + const char *prefix, + int vlan); + +/* Current, best practice */ +char * qemuBuildNicDevStr(virDomainNetDefPtr net); + +/* Both legacy & current support support */ +char *qemuBuildDriveStr(virDomainDiskDefPtr disk, + int bootable, + int qemuCmdFlags); + +/* Current, best practice */ +char * qemuBuildDriveDevStr(virConnectPtr conn, + virDomainDiskDefPtr disk); +/* Current, best practice */ +char * qemuBuildControllerDevStr(virDomainControllerDefPtr def); + +char * qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev); + +char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev); + +char * qemuBuildSoundDevStr(virDomainSoundDefPtr sound); + +char * qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev); + +char * qemuBuildChrChardevStr(virDomainChrDefPtr dev); +char * qemuBuildChrArgStr(virDomainChrDefPtr dev, const char *prefix); -char * qemuBuildDriveStr (virDomainDiskDefPtr disk, - int bootable, - int qemuCmdFlags); int qemudNetworkIfaceConnect (virConnectPtr conn, struct qemud_driver *driver, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index adf962a..bf21923 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5515,8 +5515,8 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, qemuDomainObjExitMonitorWithDriver(driver, vm); } - if (qemuBuildHostNetStr(conn, net, ' ', - net->vlan, tapfd_name, &netstr) < 0) + if (!(netstr = qemuBuildHostNetStr(conn, net, ' ', + net->vlan, tapfd_name))) goto try_tapfd_close; qemuDomainObjEnterMonitorWithDriver(driver, vm); @@ -5530,7 +5530,7 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, close(tapfd); tapfd = -1; - if (qemuBuildNicStr(conn, net, NULL, net->vlan, &nicstr) < 0) + if (!(nicstr = qemuBuildNicStr(conn, net, NULL, net->vlan))) goto try_remove; qemuDomainObjEnterMonitorWithDriver(driver, vm); -- 1.6.5.2

To allow for better code reuse from hotplug methods, the code for generating PCI/USB hostdev arg values is split out into separate methods * qemu/qemu_conf.h, qemu/qemu_conf.c: Introduce new APis for qemuBuildPCIHostdevPCIDevStr, qemuBuildUSBHostdevUsbDevStr and qemuBuildUSBHostdevDevStr --- src/qemu/qemu_conf.c | 105 +++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_conf.h | 10 +++++ 2 files changed, 81 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e11ec35..85320c1 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2508,6 +2508,65 @@ error: } +char * +qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev) +{ + char *ret = NULL; + + if (virAsprintf(&ret, "host=%.2x:%.2x.%.1x", + dev->source.subsys.u.pci.bus, + dev->source.subsys.u.pci.slot, + dev->source.subsys.u.pci.function) < 0) + virReportOOMError(NULL); + + return ret; +} + + +char * +qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev) +{ + char *ret = NULL; + + if (!dev->source.subsys.u.usb.bus && + !dev->source.subsys.u.usb.device) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("USB host device is missing bus/device information")); + return NULL; + } + + if (virAsprintf(&ret, "usb-host,hostbus=%.3d,hostaddr=%.3d,id=%s", + dev->source.subsys.u.usb.bus, + dev->source.subsys.u.usb.device, + dev->info.alias) < 0) + virReportOOMError(NULL); + + return ret; +} + + +char * +qemuBuildUSBHostdevUsbDevStr(virDomainHostdevDefPtr dev) +{ + char *ret = NULL; + + if (!dev->source.subsys.u.usb.bus && + !dev->source.subsys.u.usb.device) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("USB host device is missing bus/device information")); + return NULL; + } + + if (virAsprintf(&ret, "host:%.3d.%.3d", + dev->source.subsys.u.usb.bus, + dev->source.subsys.u.usb.device) < 0) + virReportOOMError(NULL); + + return ret; +} + + + /* This function outputs a -chardev command line option which describes only the * host side of the character device */ char * @@ -3747,10 +3806,8 @@ int qemudBuildCommandLine(virConnectPtr conn, /* Add host passthrough hardware */ for (i = 0 ; i < def->nhostdevs ; i++) { - int ret; - char* usbdev; - char* pcidev; virDomainHostdevDefPtr hostdev = def->hostdevs[i]; + char *devstr; /* USB */ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && @@ -3758,33 +3815,15 @@ int qemudBuildCommandLine(virConnectPtr conn, if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { ADD_ARG_LIT("-device"); - if (hostdev->source.subsys.u.usb.vendor) { - ret = virAsprintf(&usbdev, "usb-host,vendor=%.4x,product=%.4x,id=%s", - hostdev->source.subsys.u.usb.vendor, - hostdev->source.subsys.u.usb.product, - hostdev->info.alias); - } else { - ret = virAsprintf(&usbdev, "usb-host,hostbus=%.3d,hostaddr=%.3d,id=%s", - hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device, - hostdev->info.alias); - } + if (!(devstr = qemuBuildUSBHostdevDevStr(hostdev))) + goto error; + ADD_ARG(devstr); } else { ADD_ARG_LIT("-usbdevice"); - if (hostdev->source.subsys.u.usb.vendor) { - ret = virAsprintf(&usbdev, "host:%.4x:%.4x", - hostdev->source.subsys.u.usb.vendor, - hostdev->source.subsys.u.usb.product); - } else { - ret = virAsprintf(&usbdev, "host:%.3d.%.3d", - hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device); - } + if (!(devstr = qemuBuildUSBHostdevUsbDevStr(hostdev))) + goto error; + ADD_ARG(devstr); } - if (ret < 0) - goto error; - - ADD_ARG(usbdev); } /* PCI */ @@ -3792,21 +3831,19 @@ int qemudBuildCommandLine(virConnectPtr conn, hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { ADD_ARG_LIT("-device"); - if (!(pcidev = qemuBuildPCIHostdevDevStr(hostdev))) + if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev))) goto error; + ADD_ARG(devstr); } else if (qemuCmdFlags & QEMUD_CMD_FLAG_PCIDEVICE) { ADD_ARG_LIT("-pcidevice"); - if (virAsprintf(&pcidev, "host=%.2x:%.2x.%.1x", - hostdev->source.subsys.u.pci.bus, - hostdev->source.subsys.u.pci.slot, - hostdev->source.subsys.u.pci.function) < 0) - goto no_memory; + if (!(devstr = qemuBuildPCIHostdevPCIDevStr(hostdev))) + goto error; + ADD_ARG(devstr); } else { qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", _("PCI device assignment is not supported by this version of qemu")); goto error; } - ADD_ARG(pcidev); } } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index b6f128f..c3b196e 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -227,11 +227,21 @@ char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev); char * qemuBuildSoundDevStr(virDomainSoundDefPtr sound); +/* Legacy, pre device support */ +char * qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev); +/* Current, best practice */ char * qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev); +/* Current, best practice */ char * qemuBuildChrChardevStr(virDomainChrDefPtr dev); +/* Legacy, pre device support */ char * qemuBuildChrArgStr(virDomainChrDefPtr dev, const char *prefix); +/* Legacy, pre device support */ +char * qemuBuildUSBHostdevUsbDevStr(virDomainHostdevDefPtr dev); +/* Current, best practice */ +char * qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev); + int qemudNetworkIfaceConnect (virConnectPtr conn, -- 1.6.5.2

The way QEMU is started has been changed to use '-device' and the new style '-drive' syntax. This needs to be mirrored in the hotplug code, requiring addition of two new APIs. * src/qemu/qemu_monitor.h, src/qemu/qemu_monitor.c: Define APIs qemuMonitorAddDevice() and qemuMonitorAddDrive() * src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h, src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Implement the new monitor APIs --- src/qemu/qemu_monitor.c | 27 +++++++++++++++ src/qemu/qemu_monitor.h | 6 +++ src/qemu/qemu_monitor_json.c | 49 ++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 6 +++ src/qemu/qemu_monitor_text.c | 77 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 6 +++ 6 files changed, 171 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 817ccd7..9e09876 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1304,3 +1304,30 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, ret = qemuMonitorTextGetAllPCIAddresses(mon, addrs); return ret; } + + +int qemuMonitorAddDevice(qemuMonitorPtr mon, + const char *devicestr) +{ + DEBUG("mon=%p, fd=%d device=%s", mon, mon->fd, devicestr); + int ret; + + if (mon->json) + ret = qemuMonitorJSONAddDevice(mon, devicestr); + else + ret = qemuMonitorTextAddDevice(mon, devicestr); + return ret; +} + +int qemuMonitorAddDrive(qemuMonitorPtr mon, + const char *drivestr) +{ + DEBUG("mon=%p, fd=%d drive=%s", mon, mon->fd, drivestr); + int ret; + + if (mon->json) + ret = qemuMonitorJSONAddDrive(mon, drivestr); + else + ret = qemuMonitorTextAddDrive(mon, drivestr); + return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 8a405ce..a330eff 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -285,4 +285,10 @@ struct _qemuMonitorPCIAddress { int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, qemuMonitorPCIAddress **addrs); +int qemuMonitorAddDevice(qemuMonitorPtr mon, + const char *devicestr); + +int qemuMonitorAddDrive(qemuMonitorPtr mon, + const char *drivestr); + #endif /* QEMU_MONITOR_H */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8e88c7e..a556088 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1785,3 +1785,52 @@ int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon ATTRIBUTE_UNUSED, _("query-pci not suppported in JSON mode")); return -1; } + + +int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, + const char *devicestr) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("device_add", + "s:config", devicestr, + NULL); + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + +int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, + const char *drivestr) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("drive_add", + "s:pci_addr", "dummy", + "s:opts", drivestr, + NULL); + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 858aac0..ac6458c 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -156,4 +156,10 @@ int qemuMonitorJSONAttachDrive(qemuMonitorPtr mon, int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon, qemuMonitorPCIAddress **addrs); +int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, + const char *devicestr); + +int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, + const char *drivestr); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 380bcdc..b2a0c53 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2029,3 +2029,80 @@ error: #undef SKIP_SPACE #undef CHECK_END #undef SKIP_TO + + +int qemuMonitorTextAddDevice(qemuMonitorPtr mon, + const char *devicestr) +{ + char *cmd = NULL; + char *reply = NULL; + char *safedev; + int ret = -1; + + if (!(safedev = qemuMonitorEscapeArg(devicestr))) { + virReportOOMError(NULL); + goto cleanup; + } + + if (virAsprintf(&cmd, "device_add %s", safedev) < 0) { + virReportOOMError(NULL); + goto cleanup; + } + + if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("cannot attach %s device"), devicestr); + goto cleanup; + } + + if (STRNEQ(reply, "")) { + qemudReportError (NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("adding %s device failed: %s"), devicestr, reply); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(cmd); + VIR_FREE(reply); + return ret; +} + + +int qemuMonitorTextAddDrive(qemuMonitorPtr mon, + const char *drivestr) +{ + char *cmd = NULL; + char *reply = NULL; + int ret = -1; + char *safe_str; + + safe_str = qemuMonitorEscapeArg(drivestr); + if (!safe_str) { + virReportOOMError(NULL); + return -1; + } + + ret = virAsprintf(&cmd, "drive_add dummy %s", safe_str); + if (ret == -1) { + virReportOOMError(NULL); + goto cleanup; + } + + if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("failed to close fd in qemu with '%s'"), cmd); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(cmd); + VIR_FREE(reply); + VIR_FREE(safe_str); + return ret; +} + + diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index d6e9ca1..12d75f5 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -160,4 +160,10 @@ int qemuMonitorTextAttachDrive(qemuMonitorPtr mon, int qemuMonitorTextGetAllPCIAddresses(qemuMonitorPtr mon, qemuMonitorPCIAddress **addrs); +int qemuMonitorTextAddDevice(qemuMonitorPtr mon, + const char *devicestr); + +int qemuMonitorTextAddDrive(qemuMonitorPtr mon, + const char *drivestr); + #endif /* QEMU_MONITOR_TEXT_H */ -- 1.6.5.2

Since QEMU startup uses the new -device argument, the hotplug code needs todo the same. * src/qemu/qemu_driver.c: Use new device_add monitor APIs whereever possible --- src/qemu/qemu_driver.c | 222 ++++++++++++++++++++++++++++++++++++------------ 1 files changed, 169 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bf21923..4608a69 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5185,12 +5185,14 @@ error: static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, - virDomainDiskDefPtr disk) + virDomainDiskDefPtr disk, + int qemuCmdFlags) { int i, ret; const char* type = virDomainDiskBusTypeToString(disk->bus); qemuDomainObjPrivatePtr priv = vm->privateData; - virDomainDevicePCIAddress guestAddr; + char *devstr = NULL; + char *drivestr = NULL; for (i = 0 ; i < vm->def->ndisks ; i++) { if (STREQ(vm->def->disks[i]->dst, disk->dst)) { @@ -5205,28 +5207,52 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, driver->securityDriver->domainSetSecurityImageLabel(conn, vm, disk) < 0) return -1; + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (!(drivestr = qemuBuildDriveStr(disk, 0, qemuCmdFlags))) + goto error; + + if (!(devstr = qemuBuildDriveDevStr(NULL, disk))) + goto error; + } + if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) { virReportOOMError(conn); goto error; } qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorAddPCIDisk(priv->mon, - disk->src, - type, - &guestAddr); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ret = qemuMonitorAddDrive(priv->mon, drivestr); + if (ret == 0) + qemuMonitorAddDevice(priv->mon, devstr); + /* XXX remove the drive upon fail */ + } else { + virDomainDevicePCIAddress guestAddr; + ret = qemuMonitorAddPCIDisk(priv->mon, + disk->src, + type, + &guestAddr); + if (ret == 0) { + disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr)); + } + } qemuDomainObjExitMonitorWithDriver(driver, vm); if (ret < 0) goto error; - disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; - memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr)); virDomainDiskInsertPreAlloced(vm->def, disk); + VIR_FREE(devstr); + VIR_FREE(drivestr); + return 0; error: + VIR_FREE(devstr); + VIR_FREE(drivestr); + if (driver->securityDriver && driver->securityDriver->domainRestoreSecurityImageLabel && driver->securityDriver->domainRestoreSecurityImageLabel(conn, vm, disk) < 0) @@ -5239,10 +5265,13 @@ error: static int qemudDomainAttachPciControllerDevice(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, - virDomainControllerDefPtr controller) + virDomainControllerDefPtr controller, + int qemuCmdFlags) { - int i, ret; + int i; + int ret = -1; const char* type = virDomainControllerTypeToString(controller->type); + char *devstr = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; for (i = 0 ; i < vm->def->ncontrollers ; i++) { @@ -5255,15 +5284,24 @@ static int qemudDomainAttachPciControllerDevice(virConnectPtr conn, } } + if (!(devstr = qemuBuildControllerDevStr(controller))) { + virReportOOMError(NULL); + goto cleanup; + } + if (VIR_REALLOC_N(vm->def->controllers, vm->def->ncontrollers+1) < 0) { - virReportOOMError(conn); - return -1; + virReportOOMError(NULL); + goto cleanup; } qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorAttachPCIDiskController(priv->mon, - type, - &controller->info.addr.pci); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ret = qemuMonitorAddDevice(priv->mon, devstr); + } else { + ret = qemuMonitorAttachPCIDiskController(priv->mon, + type, + &controller->info.addr.pci); + } qemuDomainObjExitMonitorWithDriver(driver, vm); if (ret == 0) { @@ -5271,6 +5309,8 @@ static int qemudDomainAttachPciControllerDevice(virConnectPtr conn, virDomainControllerInsertPreAlloced(vm->def, controller); } +cleanup: + VIR_FREE(devstr); return ret; } @@ -5279,7 +5319,8 @@ static virDomainControllerDefPtr qemuDomainFindOrCreateSCSIDiskController(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, - int controller) + int controller, + int qemuCmdFlags) { int i; virDomainControllerDefPtr cont; @@ -5304,7 +5345,7 @@ qemuDomainFindOrCreateSCSIDiskController(virConnectPtr conn, VIR_INFO0("No SCSI controller present, hotplugging one"); if (qemudDomainAttachPciControllerDevice(conn, driver, - vm, cont) < 0) { + vm, cont, qemuCmdFlags) < 0) { VIR_FREE(cont); return NULL; } @@ -5320,9 +5361,9 @@ static int qemudDomainAttachSCSIDisk(virConnectPtr conn, { int i; qemuDomainObjPrivatePtr priv = vm->privateData; - virDomainDeviceDriveAddress driveAddr; virDomainControllerDefPtr cont; char *drivestr = NULL; + char *devstr = NULL; int ret = -1; for (i = 0 ; i < vm->def->ndisks ; i++) { @@ -5345,6 +5386,9 @@ static int qemudDomainAttachSCSIDisk(virConnectPtr conn, if (!(drivestr = qemuBuildDriveStr(disk, 0, qemuCmdFlags))) goto error; + if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && + !(devstr = qemuBuildDriveDevStr(NULL, disk))) + goto error; /* We should have an adddress now, so make sure */ if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { @@ -5355,7 +5399,7 @@ static int qemudDomainAttachSCSIDisk(virConnectPtr conn, } for (i = 0 ; i <= disk->info.addr.drive.controller ; i++) { - cont = qemuDomainFindOrCreateSCSIDiskController(conn, driver, vm, i); + cont = qemuDomainFindOrCreateSCSIDiskController(conn, driver, vm, i, qemuCmdFlags); if (!cont) goto error; } @@ -5372,26 +5416,42 @@ static int qemudDomainAttachSCSIDisk(virConnectPtr conn, } qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorAttachDrive(priv->mon, - drivestr, - &cont->info.addr.pci, - &driveAddr); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ret = qemuMonitorAddDrive(priv->mon, + drivestr); + if (ret == 0) + ret = qemuMonitorAddDevice(priv->mon, + devstr); + /* XXX should call 'drive_del' on error but this does not exist yet */ + } else { + virDomainDeviceDriveAddress driveAddr; + ret = qemuMonitorAttachDrive(priv->mon, + drivestr, + &cont->info.addr.pci, + &driveAddr); + if (ret == 0) { + /* XXX we should probably validate that the addr matches + * our existing defined addr instead of overwriting */ + disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; + memcpy(&disk->info.addr.drive, &driveAddr, sizeof(driveAddr)); + } + } qemuDomainObjExitMonitorWithDriver(driver, vm); if (ret < 0) goto error; - /* XXX we should probably validate that the addr matches - * our existing defined addr instead of overwriting */ - disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; - memcpy(&disk->info.addr.drive, &driveAddr, sizeof(driveAddr)); virDomainDiskInsertPreAlloced(vm->def, disk); + + VIR_FREE(devstr); VIR_FREE(drivestr); return 0; error: + VIR_FREE(devstr); VIR_FREE(drivestr); + if (driver->securityDriver && driver->securityDriver->domainRestoreSecurityImageLabel && driver->securityDriver->domainRestoreSecurityImageLabel(conn, vm, disk) < 0) @@ -5404,10 +5464,13 @@ error: static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, - virDomainDiskDefPtr disk) + virDomainDiskDefPtr disk, + int qemuCmdFlags) { qemuDomainObjPrivatePtr priv = vm->privateData; int i, ret; + char *drivestr = NULL; + char *devstr = NULL; for (i = 0 ; i < vm->def->ndisks ; i++) { if (STREQ(vm->def->disks[i]->dst, disk->dst)) { @@ -5428,13 +5491,29 @@ static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn, goto error; } + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (!(drivestr = qemuBuildDriveStr(disk, 0, qemuCmdFlags))) + goto error; + if (!(devstr = qemuBuildDriveDevStr(NULL, disk))) + goto error; + } + if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) { virReportOOMError(conn); goto error; } qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorAddUSBDisk(priv->mon, disk->src); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ret = qemuMonitorAddDrive(priv->mon, + drivestr); + if (ret == 0) + ret = qemuMonitorAddDevice(priv->mon, + devstr); + /* XXX should call 'drive_del' on error but this does not exist yet */ + } else { + ret = qemuMonitorAddUSBDisk(priv->mon, disk->src); + } qemuDomainObjExitMonitorWithDriver(driver, vm); if (ret < 0) @@ -5442,9 +5521,15 @@ static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn, virDomainDiskInsertPreAlloced(vm->def, disk); + VIR_FREE(devstr); + VIR_FREE(drivestr); + return 0; error: + VIR_FREE(devstr); + VIR_FREE(drivestr); + if (driver->securityDriver && driver->securityDriver->domainRestoreSecurityImageLabel && driver->securityDriver->domainRestoreSecurityImageLabel(conn, vm, disk) < 0) @@ -5587,12 +5672,14 @@ no_memory: static int qemudDomainAttachHostPciDevice(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, - virDomainHostdevDefPtr hostdev) + virDomainHostdevDefPtr hostdev, + int qemuCmdFlags) { qemuDomainObjPrivatePtr priv = vm->privateData; pciDevice *pci; int ret; virDomainDevicePCIAddress guestAddr; + char *devstr = NULL; if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) { virReportOOMError(conn); @@ -5619,10 +5706,17 @@ static int qemudDomainAttachHostPciDevice(virConnectPtr conn, return -1; } + if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && + !(devstr = qemuBuildPCIHostdevDevStr(hostdev))) + goto error; + qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorAddPCIHostDevice(priv->mon, - &hostdev->source.subsys.u.pci, - &guestAddr); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) + ret = qemuMonitorAddDevice(priv->mon, devstr); + else + ret = qemuMonitorAddPCIHostDevice(priv->mon, + &hostdev->source.subsys.u.pci, + &guestAddr); qemuDomainObjExitMonitorWithDriver(driver, vm); if (ret < 0) goto error; @@ -5631,9 +5725,12 @@ static int qemudDomainAttachHostPciDevice(virConnectPtr conn, vm->def->hostdevs[vm->def->nhostdevs++] = hostdev; + VIR_FREE(devstr); + return 0; error: + VIR_FREE(devstr); pciDeviceListDel(conn, driver->activePciHostdevs, pci); return -1; @@ -5643,39 +5740,50 @@ error: static int qemudDomainAttachHostUsbDevice(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, - virDomainHostdevDefPtr hostdev) + virDomainHostdevDefPtr hostdev, + int qemuCmdFlags) { int ret; qemuDomainObjPrivatePtr priv = vm->privateData; + char *devstr = NULL; + + if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && + !(devstr = qemuBuildPCIHostdevDevStr(hostdev))) + goto error; if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) { virReportOOMError(conn); - return -1; + goto error; } qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (hostdev->source.subsys.u.usb.vendor) { - ret = qemuMonitorAddUSBDeviceMatch(priv->mon, - hostdev->source.subsys.u.usb.vendor, - hostdev->source.subsys.u.usb.product); - } else { + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) + ret = qemuMonitorAddDevice(priv->mon, devstr); + else ret = qemuMonitorAddUSBDeviceExact(priv->mon, hostdev->source.subsys.u.usb.bus, hostdev->source.subsys.u.usb.device); - } qemuDomainObjExitMonitorWithDriver(driver, vm); + if (ret < 0) + goto error; - if (ret == 0) - vm->def->hostdevs[vm->def->nhostdevs++] = hostdev; + vm->def->hostdevs[vm->def->nhostdevs++] = hostdev; - return ret; + VIR_FREE(devstr); + + return 0; + +error: + VIR_FREE(devstr); + return -1; } static int qemudDomainAttachHostDevice(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, - virDomainHostdevDefPtr hostdev) + virDomainHostdevDefPtr hostdev, + int qemuCmdFlags) { if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT, @@ -5691,12 +5799,14 @@ static int qemudDomainAttachHostDevice(virConnectPtr conn, switch (hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: - if (qemudDomainAttachHostPciDevice(conn, driver, vm, hostdev) < 0) + if (qemudDomainAttachHostPciDevice(conn, driver, vm, + hostdev, qemuCmdFlags) < 0) goto error; break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: - if (qemudDomainAttachHostUsbDevice(conn, driver, vm, hostdev) < 0) + if (qemudDomainAttachHostUsbDevice(conn, driver, vm, + hostdev, qemuCmdFlags) < 0) goto error; break; @@ -5787,15 +5897,18 @@ static int qemudDomainAttachDevice(virDomainPtr dom, case VIR_DOMAIN_DISK_DEVICE_DISK: if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) { - ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, driver, vm, dev->data.disk); + ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, driver, vm, + dev->data.disk, qemuCmdFlags); 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->data.disk); + ret = qemudDomainAttachPciDiskDevice(dom->conn, driver, vm, + dev->data.disk, qemuCmdFlags); 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); + ret = qemudDomainAttachSCSIDisk(dom->conn, driver, vm, + dev->data.disk, qemuCmdFlags); if (ret == 0) dev->data.disk = NULL; } else { @@ -5818,7 +5931,8 @@ static int qemudDomainAttachDevice(virDomainPtr dom, } } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { - ret = qemudDomainAttachPciControllerDevice(dom->conn, driver, vm, dev->data.controller); + ret = qemudDomainAttachPciControllerDevice(dom->conn, driver, vm, + dev->data.controller, qemuCmdFlags); } else { qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, _("disk controller bus '%s' cannot be hotplugged."), @@ -5826,11 +5940,13 @@ static int qemudDomainAttachDevice(virDomainPtr dom, /* fallthrough */ } } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { - ret = qemudDomainAttachNetDevice(dom->conn, driver, vm, dev->data.net, 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->data.hostdev); + ret = qemudDomainAttachHostDevice(dom->conn, driver, vm, + dev->data.hostdev, qemuCmdFlags); if (ret == 0) dev->data.hostdev = NULL; } else { -- 1.6.5.2

The virDomainDeviceInfoIterate() function will provide a convenient way to iterate over all devices in a domain. * src/conf/domain_conf.c, src/conf/domain_conf.h, src/libvirt_private.syms: Add virDomainDeviceInfoIterate() function. --- src/conf/domain_conf.c | 67 +++++++++++++++++++++++++++++++--------------- src/conf/domain_conf.h | 8 +++++ src/libvirt_private.syms | 1 + 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e548d1d..691fc84 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -824,59 +824,82 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) } -static void virDomainDeviceInfoClearField(virDomainDeviceInfoPtr info, int alias, int pciaddr) +static int virDomainDeviceInfoClearAlias(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr info, + void *opaque ATTRIBUTE_UNUSED) { - if (alias) - VIR_FREE(info->alias); - if (pciaddr && - info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + VIR_FREE(info->alias); + return 0; +} + +static int virDomainDeviceInfoClearPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr info, + void *opaque ATTRIBUTE_UNUSED) +{ + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { memset(&info->addr, 0, sizeof(info->addr)); info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE; } + return 0; } - -static void virDomainDefClearDeviceInfo(virDomainDefPtr def, int alias, int pciaddr) +int virDomainDeviceInfoIterate(virDomainDefPtr def, + virDomainDeviceInfoCallback cb, + void *opaque) { int i; for (i = 0; i < def->ndisks ; i++) - virDomainDeviceInfoClearField(&def->disks[i]->info, alias, pciaddr); + if (cb(def, &def->disks[i]->info, opaque) < 0) + return -1; for (i = 0; i < def->nnets ; i++) - virDomainDeviceInfoClearField(&def->nets[i]->info, alias, pciaddr); + if (cb(def, &def->nets[i]->info, opaque) < 0) + return -1; for (i = 0; i < def->nsounds ; i++) - virDomainDeviceInfoClearField(&def->sounds[i]->info, alias, pciaddr); + if (cb(def, &def->sounds[i]->info, opaque) < 0) + return -1; for (i = 0; i < def->nhostdevs ; i++) - virDomainDeviceInfoClearField(&def->hostdevs[i]->info, alias, pciaddr); + if (cb(def, &def->hostdevs[i]->info, opaque) < 0) + return -1; for (i = 0; i < def->nvideos ; i++) - virDomainDeviceInfoClearField(&def->videos[i]->info, alias, pciaddr); + if (cb(def, &def->videos[i]->info, opaque) < 0) + return -1; for (i = 0; i < def->ncontrollers ; i++) - virDomainDeviceInfoClearField(&def->controllers[i]->info, alias, pciaddr); + if (cb(def, &def->controllers[i]->info, opaque) < 0) + return -1; for (i = 0; i < def->nserials ; i++) - virDomainDeviceInfoClearField(&def->serials[i]->info, alias, pciaddr); + if (cb(def, &def->serials[i]->info, opaque) < 0) + return -1; for (i = 0; i < def->nparallels ; i++) - virDomainDeviceInfoClearField(&def->parallels[i]->info, alias, pciaddr); + if (cb(def, &def->parallels[i]->info, opaque) < 0) + return -1; for (i = 0; i < def->nchannels ; i++) - virDomainDeviceInfoClearField(&def->channels[i]->info, alias, pciaddr); + if (cb(def, &def->channels[i]->info, opaque) < 0) + return -1; for (i = 0; i < def->ninputs ; i++) - virDomainDeviceInfoClearField(&def->inputs[i]->info, alias, pciaddr); + if (cb(def, &def->inputs[i]->info, opaque) < 0) + return -1; for (i = 0; i < def->nfss ; i++) - virDomainDeviceInfoClearField(&def->fss[i]->info, alias, pciaddr); + if (cb(def, &def->fss[i]->info, opaque) < 0) + return -1; if (def->watchdog) - virDomainDeviceInfoClearField(&def->watchdog->info, alias, pciaddr); + if (cb(def, &def->watchdog->info, opaque) < 0) + return -1; if (def->console) - virDomainDeviceInfoClearField(&def->console->info, alias, pciaddr); + if (cb(def, &def->console->info, opaque) < 0) + return -1; + return 0; } void virDomainDefClearPCIAddresses(virDomainDefPtr def) { - virDomainDefClearDeviceInfo(def, 0, 1); + virDomainDeviceInfoIterate(def, virDomainDeviceInfoClearPCIAddress, NULL); } void virDomainDefClearDeviceAliases(virDomainDefPtr def) { - virDomainDefClearDeviceInfo(def, 1, 0); + virDomainDeviceInfoIterate(def, virDomainDeviceInfoClearAlias, NULL); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7be090d..be0dc92 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -742,6 +742,14 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info); void virDomainDefClearPCIAddresses(virDomainDefPtr def); void virDomainDefClearDeviceAliases(virDomainDefPtr def); +typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def, + virDomainDeviceInfoPtr dev, + void *opaque); + +int virDomainDeviceInfoIterate(virDomainDefPtr def, + virDomainDeviceInfoCallback cb, + void *opaque); + void virDomainDefFree(virDomainDefPtr vm); void virDomainObjRef(virDomainObjPtr vm); /* Returns 1 if the object was freed, 0 if more refs exist */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e42c090..ce1674b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -189,6 +189,7 @@ virDomainDeviceAddressTypeToString; virDomainDefAddDiskControllers; virDomainDefClearPCIAddresses; virDomainDefClearDeviceAliases; +virDomainDeviceInfoIterate; # domain_event.h -- 1.6.5.2

The current QEMU code allocates PCI addresses incrementally starting at 4. This is not satisfactory because the user may have given some addresses in their XML config, which need to be skipped over when allocating addresses to remaining devices. It is thus neccessary to maintain a list of already allocated PCI addresses and then only allocate ones that remain unused. This is also required for domain device hotplug to work properly later. * src/qemu/qemu_conf.c, src/qemu/qemu_conf.h: Add APIs for creating list of existing PCI addresses, and allocating new addresses. Refactor address assignment to use this code * src/qemu/qemu_driver.c: Pull PCI address assignment up into the qemuStartVMDaemon() method, as a prelude to moving it into the 'define' method. Update list of allocated addresses when connecting to a running VM at daemon startup. --- src/qemu/qemu_conf.c | 228 +++++++++++++++++++++++++++++++++++++++++------- src/qemu/qemu_conf.h | 12 +++ src/qemu/qemu_driver.c | 23 +++++ 3 files changed, 233 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 85320c1..7de28be 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1587,28 +1587,174 @@ qemuAssignDeviceAliases(virDomainDefPtr def) } -static void -qemuAssignDevicePCISlot(virDomainDeviceInfoPtr info, - int slot) +#define QEMU_PCI_ADDRESS_LAST_SLOT 31 +struct _qemuDomainPCIAddressSet { + virHashTablePtr used; + int nextslot; + /* XXX add domain, bus later when QEMU allows > 1 */ +}; + + +static char *qemuPCIAddressAsString(virDomainDeviceInfoPtr dev) { - info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; - info->addr.pci.domain = 0; - info->addr.pci.bus = 0; - info->addr.pci.slot = slot; - info->addr.pci.function = 0; + char *addr; + + if (virAsprintf(&addr, "%d:%d:%d", + dev->addr.pci.domain, + dev->addr.pci.bus, + dev->addr.pci.slot) < 0) { + virReportOOMError(NULL); + return NULL; + } + return addr; } -static void -qemuAssignDevicePCISlots(virDomainDefPtr def) + +static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr dev, + void *opaque) +{ + qemuDomainPCIAddressSetPtr addrs = opaque; + + if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + char *addr = qemuPCIAddressAsString(dev); + + if (virHashAddEntry(addrs->used, addr, addr) < 0) { + VIR_FREE(addr); + return -1; + } + } + + return 0; +} + + +qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def) +{ + qemuDomainPCIAddressSetPtr addrs; + + if (VIR_ALLOC(addrs) < 0) + goto no_memory; + + if (!(addrs->used = virHashCreate(10))) + goto no_memory; + + if (virDomainDeviceInfoIterate(def, qemuCollectPCIAddress, addrs) < 0) + goto error; + + return addrs; + +no_memory: + virReportOOMError(NULL); +error: + qemuDomainPCIAddressSetFree(addrs); + return NULL; +} + +int qemuDomainPCIAddressReserve(qemuDomainPCIAddressSetPtr addrs, + int slot) +{ + virDomainDeviceInfo dev; + char *addr; + + dev.addr.pci.domain = 0; + dev.addr.pci.bus = 0; + dev.addr.pci.slot = slot; + + addr = qemuPCIAddressAsString(&dev); + if (!addr) + return -1; + + if (virHashLookup(addrs->used, addr)) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unable to reserve PCI address %s"), addr); + VIR_FREE(addr); + return -1; + } + + if (virHashAddEntry(addrs->used, addr, addr)) { + VIR_FREE(addr); + return -1; + } + + return 0; +} + + +static void qemuDomainPCIAddressSetFreeEntry(void *payload, const char *name ATTRIBUTE_UNUSED) +{ + VIR_FREE(payload); +} + + +void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs) +{ + if (!addrs) + return; + + virHashFree(addrs->used, qemuDomainPCIAddressSetFreeEntry); +} + + +int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev) { int i; - /* - * slot = 0 -> Host bridge - * slot = 1 -> PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller) - * slot = 2 -> VGA - * slot = 3 -> VirtIO Balloon - */ - int nextslot = 4; + + for (i = addrs->nextslot ; i <= QEMU_PCI_ADDRESS_LAST_SLOT ; i++) { + virDomainDeviceInfo maybe; + char *addr; + + memset(&maybe, 0, sizeof(maybe)); + maybe.addr.pci.domain = 0; + maybe.addr.pci.bus = 0; + maybe.addr.pci.slot = i; + + addr = qemuPCIAddressAsString(&maybe); + + if (virHashLookup(addrs->used, addr)) { + VIR_FREE(addr); + continue; + } + + if (virHashAddEntry(addrs->used, addr, addr) < 0) { + VIR_FREE(addr); + return -1; + } + + dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + dev->addr.pci.domain = 0; + dev->addr.pci.bus = 0; + dev->addr.pci.slot = i; + + addrs->nextslot = i + 1; + + return 0; + } + + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("No more available PCI addresses")); + return -1; +} + + +int +qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) +{ + int i; + + /* Host bridge */ + if (qemuDomainPCIAddressReserve(addrs, 0) < 0) + goto error; + /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller) */ + if (qemuDomainPCIAddressReserve(addrs, 1) < 0) + goto error; + /* VGA */ + if (qemuDomainPCIAddressReserve(addrs, 2) < 0) + goto error; + /* VirtIO Balloon */ + if (qemuDomainPCIAddressReserve(addrs, 3) < 0) + goto error; for (i = 0; i < def->ndisks ; i++) { if (def->disks[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) @@ -1618,13 +1764,15 @@ qemuAssignDevicePCISlots(virDomainDefPtr def) if (def->disks[i]->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) continue; - qemuAssignDevicePCISlot(&def->disks[i]->info, nextslot++); + if (qemuDomainPCIAddressSetNextAddr(addrs, &def->disks[i]->info) < 0) + goto error; } for (i = 0; i < def->nnets ; i++) { if (def->nets[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) continue; - qemuAssignDevicePCISlot(&def->nets[i]->info, nextslot++); + if (qemuDomainPCIAddressSetNextAddr(addrs, &def->nets[i]->info) < 0) + goto error; } for (i = 0; i < def->nsounds ; i++) { @@ -1635,7 +1783,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def) def->sounds[i]->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) continue; - qemuAssignDevicePCISlot(&def->sounds[i]->info, nextslot++); + if (qemuDomainPCIAddressSetNextAddr(addrs, &def->sounds[i]->info) < 0) + goto error; } for (i = 0; i < def->nhostdevs ; i++) { @@ -1645,14 +1794,23 @@ qemuAssignDevicePCISlots(virDomainDefPtr def) def->hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) continue; - qemuAssignDevicePCISlot(&def->hostdevs[i]->info, nextslot++); + if (qemuDomainPCIAddressSetNextAddr(addrs, &def->hostdevs[i]->info) < 0) + goto error; } for (i = 0; i < def->nvideos ; i++) { + if (def->videos[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + continue; /* First VGA is hardcoded slot=2 */ - if (i == 0) - qemuAssignDevicePCISlot(&def->videos[i]->info, 2); - else - qemuAssignDevicePCISlot(&def->videos[i]->info, nextslot++); + if (i == 0) { + def->videos[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + def->videos[i]->info.addr.pci.domain = 0; + def->videos[i]->info.addr.pci.bus = 0; + def->videos[i]->info.addr.pci.slot = 2; + def->videos[i]->info.addr.pci.function = 0; + } else { + if (qemuDomainPCIAddressSetNextAddr(addrs, &def->videos[i]->info) < 0) + goto error; + } } for (i = 0; i < def->ncontrollers ; i++) { if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) @@ -1664,10 +1822,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def) /* First IDE controller lives on the PIIX3 at slot=1, function=1 */ if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && def->controllers[i]->idx == 0) { - qemuAssignDevicePCISlot(&def->controllers[i]->info, 1); + def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + def->controllers[i]->info.addr.pci.domain = 0; + def->controllers[i]->info.addr.pci.bus = 0; + def->controllers[i]->info.addr.pci.slot = 1; def->controllers[i]->info.addr.pci.function = 1; } else { - qemuAssignDevicePCISlot(&def->controllers[i]->info, nextslot++); + if (qemuDomainPCIAddressSetNextAddr(addrs, &def->controllers[i]->info) < 0) + goto error; } } for (i = 0; i < def->ninputs ; i++) { @@ -1683,9 +1845,16 @@ qemuAssignDevicePCISlots(virDomainDefPtr def) /* Nada - none are PCI based (yet) */ /* XXX virtio-serial will need one */ } - if (def->watchdog) { - qemuAssignDevicePCISlot(&def->watchdog->info, nextslot++); + if (def->watchdog && + def->watchdog->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + if (qemuDomainPCIAddressSetNextAddr(addrs, &def->watchdog->info) < 0) + goto error; } + + return 0; + +error: + return -1; } @@ -2887,7 +3056,6 @@ int qemudBuildCommandLine(virConnectPtr conn, if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { qemuAssignDeviceAliases(def); - qemuAssignDevicePCISlots(def); } else { qemuAssignDiskAliases(def, qemuCmdFlags); } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index c3b196e..bcddf3c 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -146,6 +146,8 @@ struct qemud_driver { pciDeviceList *activePciHostdevs; }; +typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet; +typedef qemuDomainPCIAddressSet *qemuDomainPCIAddressSetPtr; /* Port numbers used for KVM migration. */ #define QEMUD_MIGRATION_FIRST_PORT 49152 @@ -272,4 +274,14 @@ virDomainDefPtr qemuParseCommandLineString(virConnectPtr conn, virCapsPtr caps, const char *args); +qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def); +int qemuDomainPCIAddressReserve(qemuDomainPCIAddressSetPtr addrs, + int slot); +int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev); +void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs); +int qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs); + + + #endif /* __QEMUD_CONF_H */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4608a69..465beaf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -93,6 +93,8 @@ struct _qemuDomainObjPrivate { int nvcpupids; int *vcpupids; + + qemuDomainPCIAddressSetPtr pciaddrs; }; static int qemudShutdown(void); @@ -146,6 +148,7 @@ static void qemuDomainObjPrivateFree(void *data) { qemuDomainObjPrivatePtr priv = data; + qemuDomainPCIAddressSetFree(priv->pciaddrs); virDomainChrDefFree(priv->monConfig); VIR_FREE(priv->vcpupids); @@ -843,11 +846,14 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq { virDomainObjPtr obj = payload; struct qemud_driver *driver = opaque; + qemuDomainObjPrivatePtr priv; virDomainObjLock(obj); VIR_DEBUG("Reconnect monitor to %p '%s'", obj, obj->def->name); + priv = obj->privateData; + /* XXX check PID liveliness & EXE path */ if (qemuConnectMonitor(obj) < 0) goto error; @@ -856,6 +862,9 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq goto error; } + if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(obj->def))) + goto error; + if (driver->securityDriver && driver->securityDriver->domainReserveSecurityLabel && driver->securityDriver->domainReserveSecurityLabel(NULL, obj) < 0) @@ -2628,6 +2637,18 @@ static int qemudStartVMDaemon(virConnectPtr conn, goto cleanup; } + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (priv->pciaddrs) { + qemuDomainPCIAddressSetFree(priv->pciaddrs); + priv->pciaddrs = NULL; + } + if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(vm->def))) + goto cleanup; + + if (qemuAssignDevicePCISlots(vm->def, priv->pciaddrs) < 0) + goto cleanup; + } + vm->def->id = driver->nextvmid++; if (qemudBuildCommandLine(conn, driver, vm->def, priv->monConfig, priv->monJSON, qemuCmdFlags, &argv, &progenv, @@ -2860,6 +2881,8 @@ static void qemudShutdownVMDaemon(virConnectPtr conn, virDomainDefClearPCIAddresses(vm->def); virDomainDefClearDeviceAliases(vm->def); + qemuDomainPCIAddressSetFree(priv->pciaddrs); + priv->pciaddrs = NULL; qemuDomainReAttachHostDevices(conn, driver, vm->def); -- 1.6.5.2

PCI disk, disk controllers, net devices and host devices need to have PCI addresses assigned before they are hot-plugged * src/qemu/qemu_conf.c: Add APIs for ensuring a device has an address and releasing unused addresses * src/qemu/qemu_driver.c: Ensure all devices have addresses when hotplugging. --- src/qemu/qemu_conf.c | 60 ++++++++++++++++++++++++++++++++++++++--------- src/qemu/qemu_conf.h | 11 +++++++- src/qemu/qemu_driver.c | 43 ++++++++++++++++++++++++++++++++-- 3 files changed, 97 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 7de28be..c93e473 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1651,17 +1651,12 @@ error: return NULL; } -int qemuDomainPCIAddressReserve(qemuDomainPCIAddressSetPtr addrs, - int slot) +int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev) { - virDomainDeviceInfo dev; char *addr; - dev.addr.pci.domain = 0; - dev.addr.pci.bus = 0; - dev.addr.pci.slot = slot; - - addr = qemuPCIAddressAsString(&dev); + addr = qemuPCIAddressAsString(dev); if (!addr) return -1; @@ -1680,6 +1675,29 @@ int qemuDomainPCIAddressReserve(qemuDomainPCIAddressSetPtr addrs, return 0; } +int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, + int slot) +{ + virDomainDeviceInfo dev; + + dev.addr.pci.domain = 0; + dev.addr.pci.bus = 0; + dev.addr.pci.slot = slot; + + return qemuDomainPCIAddressReserveAddr(addrs, &dev); +} + + +int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev) +{ + int ret = 0; + if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + ret = qemuDomainPCIAddressReserveAddr(addrs, dev); + else + ret = qemuDomainPCIAddressSetNextAddr(addrs, dev); + return ret; +} static void qemuDomainPCIAddressSetFreeEntry(void *payload, const char *name ATTRIBUTE_UNUSED) { @@ -1687,6 +1705,24 @@ static void qemuDomainPCIAddressSetFreeEntry(void *payload, const char *name ATT } +int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev) +{ + char *addr; + int ret; + + addr = qemuPCIAddressAsString(dev); + if (!addr) + return -1; + + ret = virHashRemoveEntry(addrs->used, addr, qemuDomainPCIAddressSetFreeEntry); + + VIR_FREE(addr); + + return ret; +} + + void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs) { if (!addrs) @@ -1744,16 +1780,16 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) int i; /* Host bridge */ - if (qemuDomainPCIAddressReserve(addrs, 0) < 0) + if (qemuDomainPCIAddressReserveSlot(addrs, 0) < 0) goto error; /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller) */ - if (qemuDomainPCIAddressReserve(addrs, 1) < 0) + if (qemuDomainPCIAddressReserveSlot(addrs, 1) < 0) goto error; /* VGA */ - if (qemuDomainPCIAddressReserve(addrs, 2) < 0) + if (qemuDomainPCIAddressReserveSlot(addrs, 2) < 0) goto error; /* VirtIO Balloon */ - if (qemuDomainPCIAddressReserve(addrs, 3) < 0) + if (qemuDomainPCIAddressReserveSlot(addrs, 3) < 0) goto error; for (i = 0; i < def->ndisks ; i++) { diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index bcddf3c..b94153f 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -275,10 +275,17 @@ virDomainDefPtr qemuParseCommandLineString(virConnectPtr conn, const char *args); qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def); -int qemuDomainPCIAddressReserve(qemuDomainPCIAddressSetPtr addrs, - int slot); +int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, + int slot); +int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev); int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev); +int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev); +int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev); + void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs); int qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 465beaf..8debe20 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5231,6 +5231,9 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, return -1; if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0) + goto error; + if (!(drivestr = qemuBuildDriveStr(disk, 0, qemuCmdFlags))) goto error; @@ -5276,6 +5279,11 @@ error: VIR_FREE(devstr); VIR_FREE(drivestr); + if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && + (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && + qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &disk->info) < 0) + VIR_WARN("Unable to release PCI address on %s", disk->src); + if (driver->securityDriver && driver->securityDriver->domainRestoreSecurityImageLabel && driver->securityDriver->domainRestoreSecurityImageLabel(conn, vm, disk) < 0) @@ -5307,6 +5315,10 @@ static int qemudDomainAttachPciControllerDevice(virConnectPtr conn, } } + if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && + qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &controller->info) < 0) + goto cleanup; + if (!(devstr = qemuBuildControllerDevStr(controller))) { virReportOOMError(NULL); goto cleanup; @@ -5333,6 +5345,12 @@ static int qemudDomainAttachPciControllerDevice(virConnectPtr conn, } cleanup: + if ((ret != 0) && + (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && + (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && + qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &controller->info) < 0) + VIR_WARN0("Unable to release PCI address on controller"); + VIR_FREE(devstr); return ret; } @@ -5603,6 +5621,10 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, qemuAssignNetNames(vm->def, net) < 0) goto no_memory; + if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && + qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &net->info) < 0) + goto cleanup; + /* Choose a vlan value greater than all other values since * older versions did not store the value in the state file. */ @@ -5656,6 +5678,12 @@ static int qemudDomainAttachNetDevice(virConnectPtr conn, vm->def->nets[vm->def->nnets++] = net; cleanup: + if ((ret != 0) && + (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && + (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && + qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &net->info) < 0) + VIR_WARN0("Unable to release PCI address on NIC"); + VIR_FREE(nicstr); VIR_FREE(netstr); VIR_FREE(tapfd_name); @@ -5729,9 +5757,13 @@ static int qemudDomainAttachHostPciDevice(virConnectPtr conn, return -1; } - if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && - !(devstr = qemuBuildPCIHostdevDevStr(hostdev))) - goto error; + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &hostdev->info) < 0) + goto error; + + if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev))) + goto error; + } qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) @@ -5753,6 +5785,11 @@ static int qemudDomainAttachHostPciDevice(virConnectPtr conn, return 0; error: + if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && + (hostdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && + qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &hostdev->info) < 0) + VIR_WARN0("Unable to release PCI address on host device"); + VIR_FREE(devstr); pciDeviceListDel(conn, driver->activePciHostdevs, pci); -- 1.6.5.2
participants (1)
-
Daniel P. Berrange