[libvirt] [PATCH 0/7] More QEMU_CAPS_DEVICE cleanups
While looking at Laine's patches I found uses of ADDRESS_TYPE_PCI in functions that are only called on !QEMU_CAPS_DEVICE (i.e. never). Then I cleaned up some more code until I remembered Cole already started doing that. So I included the only patch I could apply easily. Cole Robinson (1): qemu: process: Drop !QEMU_CAPS_DEVICE code Ján Tomko (6): Remove qemuProcessInitPCIAddresses with dependencies qemu: always add -nodefaults Remove DISK_BUS_XEN support from qemuBuildDiskDriveCommandLine Assume QEMU_CAPS_DEVICE in qemuBuildDiskDriveCommandLine Introduce qemuDiskBusNeedsDeviceArg Do not mask QEMU_CAPS_DEVICE in qemuBuildDriveStr src/qemu/qemu_command.c | 90 ++-- src/qemu/qemu_monitor.c | 15 - src/qemu/qemu_monitor.h | 10 - src/qemu/qemu_monitor_json.c | 9 - src/qemu/qemu_monitor_json.h | 3 - src/qemu/qemu_monitor_text.c | 126 ------ src/qemu/qemu_monitor_text.h | 3 - src/qemu/qemu_process.c | 463 ++------------------- .../qemuargv2xmldata/qemuargv2xml-disk-xenvbd.args | 25 -- .../qemuargv2xmldata/qemuargv2xml-disk-xenvbd.xml | 51 --- tests/qemuargv2xmltest.c | 1 - .../qemuxml2argvdata/qemuxml2argv-disk-xenvbd.args | 26 -- .../qemuxml2argvdata/qemuxml2argv-disk-xenvbd.xml | 47 --- tests/qemuxml2argvtest.c | 1 - .../qemuxml2xmlout-disk-xenvbd.xml | 51 --- tests/qemuxml2xmltest.c | 1 - 16 files changed, 48 insertions(+), 874 deletions(-) delete mode 100644 tests/qemuargv2xmldata/qemuargv2xml-disk-xenvbd.args delete mode 100644 tests/qemuargv2xmldata/qemuargv2xml-disk-xenvbd.xml delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.xml delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-xenvbd.xml -- 2.7.3
It was only called for QEMUs without QEMU_CAPS_DEVICE, which we no longer support. --- src/qemu/qemu_monitor.c | 15 -- src/qemu/qemu_monitor.h | 10 -- src/qemu/qemu_monitor_json.c | 9 - src/qemu/qemu_monitor_json.h | 3 - src/qemu/qemu_monitor_text.c | 126 ------------- src/qemu/qemu_monitor_text.h | 3 - src/qemu/qemu_process.c | 410 ------------------------------------------- 7 files changed, 576 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1f402ee..7d1ca91 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2695,21 +2695,6 @@ qemuMonitorGetChardevInfo(qemuMonitorPtr mon, } -int -qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, - qemuMonitorPCIAddress **addrs) -{ - VIR_DEBUG("addrs=%p", addrs); - - QEMU_CHECK_MONITOR(mon); - - if (mon->json) - return qemuMonitorJSONGetAllPCIAddresses(mon, addrs); - else - return qemuMonitorTextGetAllPCIAddresses(mon, addrs); -} - - /** * qemuMonitorDriveDel: * @mon: monitor object diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 4d8f21f..6359314 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -677,16 +677,6 @@ int qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, virPCIDeviceAddress *guestAddr); -typedef struct _qemuMonitorPCIAddress qemuMonitorPCIAddress; -struct _qemuMonitorPCIAddress { - unsigned int vendor; - unsigned int product; - virPCIDeviceAddress addr; -}; - -int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, - qemuMonitorPCIAddress **addrs); - int qemuMonitorAddDevice(qemuMonitorPtr mon, const char *devicestr); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 1790e4d..64a8765 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3568,15 +3568,6 @@ qemuMonitorJSONGetChardevInfo(qemuMonitorPtr mon, } -int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon ATTRIBUTE_UNUSED, - qemuMonitorPCIAddress **addrs ATTRIBUTE_UNUSED) -{ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-pci not supported in JSON mode")); - return -1; -} - - int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, const char *devalias) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index b207e3c..95bba69 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -215,9 +215,6 @@ int qemuMonitorJSONAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, virPCIDeviceAddress *guestAddr); -int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon, - qemuMonitorPCIAddress **addrs); - int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, const char *devicestr); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index ddf16b2..28a6e1b 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1833,132 +1833,6 @@ int qemuMonitorTextGetChardevInfo(qemuMonitorPtr mon, } -/* - * The format we're after looks like this - * - * (qemu) info pci - * Bus 0, device 0, function 0: - * Host bridge: PCI device 8086:1237 - * id "" - * Bus 0, device 1, function 0: - * ISA bridge: PCI device 8086:7000 - * id "" - * Bus 0, device 1, function 1: - * IDE controller: PCI device 8086:7010 - * BAR4: I/O at 0xc000 [0xc00f]. - * id "" - * Bus 0, device 1, function 3: - * Bridge: PCI device 8086:7113 - * IRQ 9. - * id "" - * Bus 0, device 2, function 0: - * VGA controller: PCI device 1013:00b8 - * BAR0: 32 bit prefetchable memory at 0xf0000000 [0xf1ffffff]. - * BAR1: 32 bit memory at 0xf2000000 [0xf2000fff]. - * id "" - * Bus 0, device 3, function 0: - * Ethernet controller: PCI device 8086:100e - * IRQ 11. - * BAR0: 32 bit memory at 0xf2020000 [0xf203ffff]. - * BAR1: I/O at 0xc040 [0xc07f]. - * id "" - * - * Of this, we're interesting in the vendor/product ID - * and the bus/device/function data. - */ -#define CHECK_END(p) if (!(p)) break; -#define SKIP_TO(p, lbl) \ - (p) = strstr((p), (lbl)); \ - if (p) \ - (p) += strlen(lbl); -#define GET_INT(p, base, val) \ - if (virStrToLong_ui((p), &(p), (base), &(val)) < 0) { \ - virReportError(VIR_ERR_OPERATION_FAILED, \ - _("cannot parse value for %s"), #val); \ - break; \ - } -#define SKIP_SPACE(p) \ - while (*(p) == ' ') (p)++; - -int qemuMonitorTextGetAllPCIAddresses(qemuMonitorPtr mon, - qemuMonitorPCIAddress **retaddrs) -{ - char *reply; - qemuMonitorPCIAddress *addrs = NULL; - int naddrs = 0; - char *p; - - *retaddrs = NULL; - - if (qemuMonitorHMPCommand(mon, "info pci", &reply) < 0) - return -1; - - p = reply; - - - while (p) { - unsigned int bus, slot, func, vendor, product; - - SKIP_TO(p, " Bus"); - CHECK_END(p); - SKIP_SPACE(p); - GET_INT(p, 10, bus); - CHECK_END(p); - - SKIP_TO(p, ", device"); - CHECK_END(p); - SKIP_SPACE(p); - GET_INT(p, 10, slot); - CHECK_END(p); - - SKIP_TO(p, ", function"); - CHECK_END(p); - SKIP_SPACE(p); - GET_INT(p, 10, func); - CHECK_END(p); - - SKIP_TO(p, "PCI device"); - CHECK_END(p); - SKIP_SPACE(p); - GET_INT(p, 16, vendor); - CHECK_END(p); - - if (*p != ':') - break; - p++; - GET_INT(p, 16, product); - - if (VIR_REALLOC_N(addrs, naddrs+1) < 0) - goto error; - - addrs[naddrs].addr.domain = 0; - addrs[naddrs].addr.bus = bus; - addrs[naddrs].addr.slot = slot; - addrs[naddrs].addr.function = func; - addrs[naddrs].vendor = vendor; - addrs[naddrs].product = product; - naddrs++; - - VIR_DEBUG("Got dev %d:%d:%d %x:%x", bus, slot, func, vendor, product); - } - - VIR_FREE(reply); - - *retaddrs = addrs; - - return naddrs; - - error: - VIR_FREE(addrs); - VIR_FREE(reply); - return -1; -} -#undef GET_INT -#undef SKIP_SPACE -#undef CHECK_END -#undef SKIP_TO - - int qemuMonitorTextDelDevice(qemuMonitorPtr mon, const char *devalias) { diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index d30f1a4..eeaca52 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -142,9 +142,6 @@ int qemuMonitorTextRemoveNetdev(qemuMonitorPtr mon, int qemuMonitorTextGetChardevInfo(qemuMonitorPtr mon, virHashTablePtr info); -int qemuMonitorTextGetAllPCIAddresses(qemuMonitorPtr mon, - qemuMonitorPCIAddress **addrs); - int qemuMonitorTextAddDevice(qemuMonitorPtr mon, const char *devicestr); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f4bf6c1..40c238b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2441,400 +2441,6 @@ qemuProcessInitPasswords(virConnectPtr conn, } -#define QEMU_PCI_VENDOR_INTEL 0x8086 -#define QEMU_PCI_VENDOR_LSI_LOGIC 0x1000 -#define QEMU_PCI_VENDOR_REDHAT 0x1af4 -#define QEMU_PCI_VENDOR_CIRRUS 0x1013 -#define QEMU_PCI_VENDOR_REALTEK 0x10ec -#define QEMU_PCI_VENDOR_AMD 0x1022 -#define QEMU_PCI_VENDOR_ENSONIQ 0x1274 -#define QEMU_PCI_VENDOR_VMWARE 0x15ad -#define QEMU_PCI_VENDOR_QEMU 0x1234 - -#define QEMU_PCI_PRODUCT_DISK_VIRTIO 0x1001 - -#define QEMU_PCI_PRODUCT_BALLOON_VIRTIO 0x1002 - -#define QEMU_PCI_PRODUCT_NIC_NE2K 0x8029 -#define QEMU_PCI_PRODUCT_NIC_PCNET 0x2000 -#define QEMU_PCI_PRODUCT_NIC_RTL8139 0x8139 -#define QEMU_PCI_PRODUCT_NIC_E1000 0x100E -#define QEMU_PCI_PRODUCT_NIC_VIRTIO 0x1000 - -#define QEMU_PCI_PRODUCT_VGA_CIRRUS 0x00b8 -#define QEMU_PCI_PRODUCT_VGA_VMWARE 0x0405 -#define QEMU_PCI_PRODUCT_VGA_STDVGA 0x1111 - -#define QEMU_PCI_PRODUCT_AUDIO_AC97 0x2415 -#define QEMU_PCI_PRODUCT_AUDIO_ES1370 0x5000 - -#define QEMU_PCI_PRODUCT_CONTROLLER_PIIX 0x7010 -#define QEMU_PCI_PRODUCT_CONTROLLER_LSI 0x0012 - -#define QEMU_PCI_PRODUCT_WATCHDOG_I63000ESB 0x25ab - -static int -qemuProcessAssignNextPCIAddress(virDomainDeviceInfo *info, - int vendor, - int product, - qemuMonitorPCIAddress *addrs, - int naddrs) -{ - bool found = false; - size_t i; - - VIR_DEBUG("Look for %x:%x out of %d", vendor, product, naddrs); - - for (i = 0; i < naddrs; i++) { - VIR_DEBUG("Maybe %x:%x", addrs[i].vendor, addrs[i].product); - if (addrs[i].vendor == vendor && - addrs[i].product == product) { - VIR_DEBUG("Match %zu", i); - found = true; - break; - } - } - if (!found) - return -1; - - /* Blank it out so this device isn't matched again */ - addrs[i].vendor = 0; - addrs[i].product = 0; - - if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) - info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; - - if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - info->addr.pci.domain = addrs[i].addr.domain; - info->addr.pci.bus = addrs[i].addr.bus; - info->addr.pci.slot = addrs[i].addr.slot; - info->addr.pci.function = addrs[i].addr.function; - } - - return 0; -} - -static int -qemuProcessGetPCIDiskVendorProduct(virDomainDiskDefPtr def, - unsigned *vendor, - unsigned *product) -{ - switch (def->bus) { - case VIR_DOMAIN_DISK_BUS_VIRTIO: - *vendor = QEMU_PCI_VENDOR_REDHAT; - *product = QEMU_PCI_PRODUCT_DISK_VIRTIO; - break; - - default: - return -1; - } - - return 0; -} - -static int -qemuProcessGetPCINetVendorProduct(virDomainNetDefPtr def, - unsigned *vendor, - unsigned *product) -{ - if (!def->model) - return -1; - - if (STREQ(def->model, "ne2k_pci")) { - *vendor = QEMU_PCI_VENDOR_REALTEK; - *product = QEMU_PCI_PRODUCT_NIC_NE2K; - } else if (STREQ(def->model, "pcnet")) { - *vendor = QEMU_PCI_VENDOR_AMD; - *product = QEMU_PCI_PRODUCT_NIC_PCNET; - } else if (STREQ(def->model, "rtl8139")) { - *vendor = QEMU_PCI_VENDOR_REALTEK; - *product = QEMU_PCI_PRODUCT_NIC_RTL8139; - } else if (STREQ(def->model, "e1000")) { - *vendor = QEMU_PCI_VENDOR_INTEL; - *product = QEMU_PCI_PRODUCT_NIC_E1000; - } else if (STREQ(def->model, "virtio")) { - *vendor = QEMU_PCI_VENDOR_REDHAT; - *product = QEMU_PCI_PRODUCT_NIC_VIRTIO; - } else { - VIR_INFO("Unexpected NIC model %s, cannot get PCI address", - def->model); - return -1; - } - return 0; -} - -static int -qemuProcessGetPCIControllerVendorProduct(virDomainControllerDefPtr def, - unsigned *vendor, - unsigned *product) -{ - switch (def->type) { - case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: - *vendor = QEMU_PCI_VENDOR_LSI_LOGIC; - *product = QEMU_PCI_PRODUCT_CONTROLLER_LSI; - break; - - case VIR_DOMAIN_CONTROLLER_TYPE_FDC: - /* XXX we could put in the ISA bridge address, but - that's not technically the FDC's address */ - return -1; - - case VIR_DOMAIN_CONTROLLER_TYPE_IDE: - *vendor = QEMU_PCI_VENDOR_INTEL; - *product = QEMU_PCI_PRODUCT_CONTROLLER_PIIX; - break; - - default: - VIR_INFO("Unexpected controller type %s, cannot get PCI address", - virDomainControllerTypeToString(def->type)); - return -1; - } - - return 0; -} - -static int -qemuProcessGetPCIVideoVendorProduct(virDomainVideoDefPtr def, - unsigned *vendor, - unsigned *product) -{ - switch (def->type) { - case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: - *vendor = QEMU_PCI_VENDOR_CIRRUS; - *product = QEMU_PCI_PRODUCT_VGA_CIRRUS; - break; - - case VIR_DOMAIN_VIDEO_TYPE_VGA: - *vendor = QEMU_PCI_VENDOR_QEMU; - *product = QEMU_PCI_PRODUCT_VGA_STDVGA; - break; - - case VIR_DOMAIN_VIDEO_TYPE_VMVGA: - *vendor = QEMU_PCI_VENDOR_VMWARE; - *product = QEMU_PCI_PRODUCT_VGA_VMWARE; - break; - - default: - return -1; - } - return 0; -} - -static int -qemuProcessGetPCISoundVendorProduct(virDomainSoundDefPtr def, - unsigned *vendor, - unsigned *product) -{ - switch (def->model) { - case VIR_DOMAIN_SOUND_MODEL_ES1370: - *vendor = QEMU_PCI_VENDOR_ENSONIQ; - *product = QEMU_PCI_PRODUCT_AUDIO_ES1370; - break; - - case VIR_DOMAIN_SOUND_MODEL_AC97: - *vendor = QEMU_PCI_VENDOR_INTEL; - *product = QEMU_PCI_PRODUCT_AUDIO_AC97; - break; - - default: - return -1; - } - - return 0; -} - -static int -qemuProcessGetPCIWatchdogVendorProduct(virDomainWatchdogDefPtr def, - unsigned *vendor, - unsigned *product) -{ - switch (def->model) { - case VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB: - *vendor = QEMU_PCI_VENDOR_INTEL; - *product = QEMU_PCI_PRODUCT_WATCHDOG_I63000ESB; - break; - - default: - return -1; - } - - return 0; -} - - -static int -qemuProcessGetPCIMemballoonVendorProduct(virDomainMemballoonDefPtr def, - unsigned *vendor, - unsigned *product) -{ - switch (def->model) { - case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO: - *vendor = QEMU_PCI_VENDOR_REDHAT; - *product = QEMU_PCI_PRODUCT_BALLOON_VIRTIO; - break; - - default: - return -1; - } - - return 0; -} - - -/* - * This entire method assumes that PCI devices in 'info pci' - * match ordering of devices specified on the command line - * wrt to devices of matching vendor+product - * - * XXXX this might not be a valid assumption if we assign - * some static addrs on CLI. Have to check that... - */ -static int -qemuProcessDetectPCIAddresses(virDomainObjPtr vm, - qemuMonitorPCIAddress *addrs, - int naddrs) -{ - unsigned int vendor = 0, product = 0; - size_t i; - - /* XXX should all these vendor/product IDs be kept in the - * actual device data structure instead ? - */ - - for (i = 0; i < vm->def->ndisks; i++) { - if (qemuProcessGetPCIDiskVendorProduct(vm->def->disks[i], &vendor, &product) < 0) - continue; - - if (qemuProcessAssignNextPCIAddress(&(vm->def->disks[i]->info), - vendor, product, - addrs, naddrs) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find PCI address for VirtIO disk %s"), - vm->def->disks[i]->dst); - return -1; - } - } - - for (i = 0; i < vm->def->nnets; i++) { - if (qemuProcessGetPCINetVendorProduct(vm->def->nets[i], &vendor, &product) < 0) - continue; - - if (qemuProcessAssignNextPCIAddress(&(vm->def->nets[i]->info), - vendor, product, - addrs, naddrs) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find PCI address for %s NIC"), - vm->def->nets[i]->model); - return -1; - } - } - - for (i = 0; i < vm->def->ncontrollers; i++) { - if (qemuProcessGetPCIControllerVendorProduct(vm->def->controllers[i], &vendor, &product) < 0) - continue; - - if (qemuProcessAssignNextPCIAddress(&(vm->def->controllers[i]->info), - vendor, product, - addrs, naddrs) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find PCI address for controller %s"), - virDomainControllerTypeToString(vm->def->controllers[i]->type)); - return -1; - } - } - - for (i = 0; i < vm->def->nvideos; i++) { - if (qemuProcessGetPCIVideoVendorProduct(vm->def->videos[i], &vendor, &product) < 0) - continue; - - if (qemuProcessAssignNextPCIAddress(&(vm->def->videos[i]->info), - vendor, product, - addrs, naddrs) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find PCI address for video adapter %s"), - virDomainVideoTypeToString(vm->def->videos[i]->type)); - return -1; - } - } - - for (i = 0; i < vm->def->nsounds; i++) { - if (qemuProcessGetPCISoundVendorProduct(vm->def->sounds[i], &vendor, &product) < 0) - continue; - - if (qemuProcessAssignNextPCIAddress(&(vm->def->sounds[i]->info), - vendor, product, - addrs, naddrs) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find PCI address for sound adapter %s"), - virDomainSoundModelTypeToString(vm->def->sounds[i]->model)); - return -1; - } - } - - - if (vm->def->watchdog && - qemuProcessGetPCIWatchdogVendorProduct(vm->def->watchdog, &vendor, &product) == 0) { - if (qemuProcessAssignNextPCIAddress(&(vm->def->watchdog->info), - vendor, product, - addrs, naddrs) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find PCI address for watchdog %s"), - virDomainWatchdogModelTypeToString(vm->def->watchdog->model)); - return -1; - } - } - - if (vm->def->memballoon && - qemuProcessGetPCIMemballoonVendorProduct(vm->def->memballoon, &vendor, &product) == 0) { - if (qemuProcessAssignNextPCIAddress(&(vm->def->memballoon->info), - vendor, product, - addrs, naddrs) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find PCI address for balloon %s"), - virDomainMemballoonModelTypeToString(vm->def->memballoon->model)); - return -1; - } - } - - /* XXX console (virtio) */ - - - /* ... and now things we don't have in our xml */ - - /* XXX USB controller ? */ - - /* XXX what about other PCI devices (ie bridges) */ - - return 0; -} - -static int -qemuProcessInitPCIAddresses(virQEMUDriverPtr driver, - virDomainObjPtr vm, - int asyncJob) -{ - qemuDomainObjPrivatePtr priv = vm->privateData; - int naddrs; - int ret = -1; - qemuMonitorPCIAddress *addrs = NULL; - - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - return -1; - naddrs = qemuMonitorGetAllPCIAddresses(priv->mon, - &addrs); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto cleanup; - - if (naddrs > 0) - ret = qemuProcessDetectPCIAddresses(vm, addrs, naddrs); - - cleanup: - VIR_FREE(addrs); - - return ret; -} - - static int qemuProcessPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainChrDefPtr dev, @@ -5616,14 +5222,6 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuProcessInitPasswords(conn, driver, vm, asyncJob) < 0) goto cleanup; - /* If we have -device, then addresses are assigned explicitly. - * If not, then we have to detect dynamic ones here */ - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - VIR_DEBUG("Determining domain device PCI addresses"); - if (qemuProcessInitPCIAddresses(driver, vm, asyncJob) < 0) - goto cleanup; - } - /* set default link states */ /* qemu doesn't support setting this on the command line, so * enter the monitor */ @@ -6420,14 +6018,6 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, if (qemuProcessDetectIOThreadPIDs(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) goto error; - /* If we have -device, then addresses are assigned explicitly. - * If not, then we have to detect dynamic ones here */ - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - VIR_DEBUG("Determining domain device PCI addresses"); - if (qemuProcessInitPCIAddresses(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) - goto error; - } - VIR_DEBUG("Getting initial memory amount"); qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorGetBalloonInfo(priv->mon, &vm->def->mem.cur_balloon) < 0) -- 2.7.3
From: Cole Robinson <crobinso@redhat.com> Nowadays we only support qemu 0.12.0+ which provides QEMU_CAPS_DEVICE, so this is all dead code. --- src/qemu/qemu_process.c | 53 ++++++++++++++++++++++--------------------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 40c238b..b7c8f25 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2406,31 +2406,29 @@ qemuProcessInitPasswords(virConnectPtr conn, goto cleanup; } - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - for (i = 0; i < vm->def->ndisks; i++) { - size_t secretLen; + for (i = 0; i < vm->def->ndisks; i++) { + size_t secretLen; - if (!vm->def->disks[i]->src->encryption || - !virDomainDiskGetSource(vm->def->disks[i])) - continue; + if (!vm->def->disks[i]->src->encryption || + !virDomainDiskGetSource(vm->def->disks[i])) + continue; - VIR_FREE(secret); - if (qemuProcessGetVolumeQcowPassphrase(conn, - vm->def->disks[i], - &secret, &secretLen) < 0) - goto cleanup; + VIR_FREE(secret); + if (qemuProcessGetVolumeQcowPassphrase(conn, + vm->def->disks[i], + &secret, &secretLen) < 0) + goto cleanup; - VIR_FREE(alias); - if (VIR_STRDUP(alias, vm->def->disks[i]->info.alias) < 0) - goto cleanup; - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) - goto cleanup; - ret = qemuMonitorSetDrivePassphrase(priv->mon, alias, secret); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - ret = -1; - if (ret < 0) - goto cleanup; - } + VIR_FREE(alias); + if (VIR_STRDUP(alias, vm->def->disks[i]->info.alias) < 0) + goto cleanup; + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + goto cleanup; + ret = qemuMonitorSetDrivePassphrase(priv->mon, alias, secret); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + if (ret < 0) + goto cleanup; } cleanup: @@ -3290,9 +3288,8 @@ qemuProcessReconnect(void *opaque) goto cleanup; } - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) - if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps, obj)) < 0) - goto error; + if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps, obj)) < 0) + goto error; /* if domain requests security driver we haven't loaded, report error, but * do not kill the domain @@ -5981,11 +5978,9 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, * we also need to populate the PCI address set cache for later * use in hotplug */ - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - VIR_DEBUG("Assigning domain PCI addresses"); - if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0) + VIR_DEBUG("Assigning domain PCI addresses"); + if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0) goto error; - } if ((timestamp = virTimeStringNow()) == NULL) goto error; -- 2.7.3
On 05/19/2016 02:59 PM, Ján Tomko wrote:
From: Cole Robinson <crobinso@redhat.com>
Nowadays we only support qemu 0.12.0+ which provides QEMU_CAPS_DEVICE, so this is all dead code. --- src/qemu/qemu_process.c | 53 ++++++++++++++++++++++--------------------------- 1 file changed, 24 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 40c238b..b7c8f25 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c
[...]
@@ -5981,11 +5978,9 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, * we also need to populate the PCI address set cache for later * use in hotplug */ - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - VIR_DEBUG("Assigning domain PCI addresses"); - if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0) + VIR_DEBUG("Assigning domain PCI addresses"); + if ((qemuDomainAssignAddresses(vm->def, priv->qemuCaps, vm)) < 0) goto error;
There's 4 extra spaces on indention for goto error. John
Since we always asumme support of QEMU_CAPS_DEVICE. --- src/qemu/qemu_command.c | 37 ++++++++----------------------------- 1 file changed, 8 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4a2fa1d..6c1bd8f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4213,13 +4213,8 @@ qemuBuildVideoCommandLine(virCommandPtr cmd, size_t i; int primaryVideoType; - if (!def->nvideos) { - /* If we have -device, then we set -nodefaults already */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_NONE)) - virCommandAddArgList(cmd, "-vga", "none", NULL); + if (!def->videos) return 0; - } primaryVideoType = def->videos[0]->type; @@ -8223,11 +8218,7 @@ qemuBuildNetCommandLine(virCommandPtr cmd, int last_good_net = -1; virErrorPtr originalError = NULL; - if (!def->nnets) { - /* If we have -device, then we set -nodefault already */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-net", "none", NULL); - } else { + if (def->nnets) { unsigned int bootNet = 0; if (emitBootindex) { @@ -8545,7 +8536,6 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager, virQEMUCapsPtr qemuCaps) { size_t i; - int actualSerials = 0; bool havespice = false; if (def->nserials) { @@ -8582,13 +8572,8 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager, virCommandAddArg(cmd, devstr); VIR_FREE(devstr); } - actualSerials++; } - /* If we have -device, then we set -nodefaults already */ - if (!actualSerials && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-serial", "none", NULL); - return 0; } @@ -8601,10 +8586,6 @@ qemuBuildParallelsCommandLine(virLogManagerPtr logManager, { size_t i; - /* If we have -device, then we set -nodefaults already */ - if (!def->nparallels && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-parallel", "none", NULL); - for (i = 0; i < def->nparallels; i++) { virDomainChrDefPtr parallel = def->parallels[i]; char *devstr; @@ -9477,14 +9458,12 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); } - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - /* Disable global config files and default devices */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_USER_CONFIG)) - virCommandAddArg(cmd, "-no-user-config"); - else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NODEFCONFIG)) - virCommandAddArg(cmd, "-nodefconfig"); - virCommandAddArg(cmd, "-nodefaults"); - } + /* Disable global config files and default devices */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_USER_CONFIG)) + virCommandAddArg(cmd, "-no-user-config"); + else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NODEFCONFIG)) + virCommandAddArg(cmd, "-nodefconfig"); + virCommandAddArg(cmd, "-nodefaults"); if (qemuBuildSgaCommandLine(cmd, def, qemuCaps) < 0) goto error; -- 2.7.3
We have stopped supporting Xenner some time ago. --- src/qemu/qemu_command.c | 5 +-- .../qemuargv2xmldata/qemuargv2xml-disk-xenvbd.args | 25 ----------- .../qemuargv2xmldata/qemuargv2xml-disk-xenvbd.xml | 51 ---------------------- tests/qemuargv2xmltest.c | 1 - .../qemuxml2argvdata/qemuxml2argv-disk-xenvbd.args | 26 ----------- .../qemuxml2argvdata/qemuxml2argv-disk-xenvbd.xml | 47 -------------------- tests/qemuxml2argvtest.c | 1 - .../qemuxml2xmlout-disk-xenvbd.xml | 51 ---------------------- tests/qemuxml2xmltest.c | 1 - 9 files changed, 2 insertions(+), 206 deletions(-) delete mode 100644 tests/qemuargv2xmldata/qemuargv2xml-disk-xenvbd.args delete mode 100644 tests/qemuargv2xmldata/qemuargv2xml-disk-xenvbd.xml delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.xml delete mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-xenvbd.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6c1bd8f..bb40c17 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1951,13 +1951,12 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, "-drive"); /* Unfortunately it is not possible to use - -device for floppies, xen PV, or SD + -device for floppies, or SD devices. Fortunately, those don't need static PCI addresses, so we don't really care that we can't use -device */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (disk->bus != VIR_DOMAIN_DISK_BUS_XEN && - disk->bus != VIR_DOMAIN_DISK_BUS_SD) { + if (disk->bus != VIR_DOMAIN_DISK_BUS_SD) { withDeviceArg = true; } else { virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE); diff --git a/tests/qemuargv2xmldata/qemuargv2xml-disk-xenvbd.args b/tests/qemuargv2xmldata/qemuargv2xml-disk-xenvbd.args deleted file mode 100644 index 07fb4e4..0000000 --- a/tests/qemuargv2xmldata/qemuargv2xml-disk-xenvbd.args +++ /dev/null @@ -1,25 +0,0 @@ -LC_ALL=C \ -PATH=/bin \ -HOME=/home/test \ -USER=test \ -LOGNAME=test \ -QEMU_AUDIO_DRV=none \ -/usr/bin/qemu \ --name QEMUGuest1 \ --S \ --M pc \ --m 214 \ --smp 1 \ --uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ --nographic \ --monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi \ --boot c \ --usb \ --drive file=/dev/HostVG/QEMUGuest1,format=raw,if=ide,bus=0,unit=0 \ --drive file=/dev/HostVG/QEMUGuest2,format=raw,if=ide,media=cdrom,bus=1,unit=0 \ --drive file=/tmp/data.img,format=raw,if=xen,index=0 \ --drive file=/tmp/logs.img,format=raw,if=xen,index=6 \ --net none \ --serial none \ --parallel none diff --git a/tests/qemuargv2xmldata/qemuargv2xml-disk-xenvbd.xml b/tests/qemuargv2xmldata/qemuargv2xml-disk-xenvbd.xml deleted file mode 100644 index 17c5e2c..0000000 --- a/tests/qemuargv2xmldata/qemuargv2xml-disk-xenvbd.xml +++ /dev/null @@ -1,51 +0,0 @@ -<domain type='qemu'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219136</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='i686' machine='pc'>hvm</type> - <boot dev='hd'/> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu</emulator> - <disk type='block' device='disk'> - <driver name='qemu' type='raw'/> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='ide'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> - <disk type='block' device='cdrom'> - <driver name='qemu' type='raw'/> - <source dev='/dev/HostVG/QEMUGuest2'/> - <target dev='hdc' bus='ide'/> - <readonly/> - <address type='drive' controller='0' bus='1' target='0' unit='0'/> - </disk> - <disk type='file' device='disk'> - <driver name='qemu' type='raw'/> - <source file='/tmp/data.img'/> - <target dev='xvda' bus='xen'/> - </disk> - <disk type='file' device='disk'> - <driver name='qemu' type='raw'/> - <source file='/tmp/logs.img'/> - <target dev='xvdg' bus='xen'/> - </disk> - <controller type='usb' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> - </controller> - <controller type='pci' index='0' model='pci-root'/> - <controller type='ide' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <memballoon model='none'/> - </devices> -</domain> diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 48c83ea..73d3f5c 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -195,7 +195,6 @@ mymain(void) DO_TEST("disk-floppy"); DO_TEST("disk-many"); DO_TEST("disk-virtio"); - DO_TEST("disk-xenvbd"); DO_TEST("disk-drive-boot-disk"); DO_TEST("disk-drive-boot-cdrom"); DO_TEST("disk-drive-fmt-qcow"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.args deleted file mode 100644 index 04b9be6..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.args +++ /dev/null @@ -1,26 +0,0 @@ -LC_ALL=C \ -PATH=/bin \ -HOME=/home/test \ -USER=test \ -LOGNAME=test \ -QEMU_AUDIO_DRV=none \ -/usr/bin/qemu \ --name QEMUGuest1 \ --S \ --M pc \ --m 214 \ --smp 1 \ --uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ --nographic \ --nodefaults \ --monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ --no-acpi \ --boot c \ --usb \ --drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ --device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ --drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,media=cdrom,\ -id=drive-ide0-1-0 \ --device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ --drive file=/tmp/data.img,format=raw,if=xen,index=0 \ --drive file=/tmp/logs.img,format=raw,if=xen,index=6 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.xml deleted file mode 100644 index 088daff..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.xml +++ /dev/null @@ -1,47 +0,0 @@ -<domain type='qemu'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219136</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='i686' machine='pc'>hvm</type> - <boot dev='hd'/> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu</emulator> - <disk type='block' device='disk'> - <driver name='qemu' type='raw'/> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='ide'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> - <disk type='block' device='cdrom'> - <driver name='qemu' type='raw'/> - <source dev='/dev/HostVG/QEMUGuest2'/> - <target dev='hdc' bus='ide'/> - <readonly/> - <address type='drive' controller='0' bus='1' target='0' unit='0'/> - </disk> - <disk type='file' device='disk'> - <driver name='qemu' type='raw'/> - <source file='/tmp/data.img'/> - <target dev='xvda' bus='xen'/> - </disk> - <disk type='file' device='disk'> - <driver name='qemu' type='raw'/> - <source file='/tmp/logs.img'/> - <target dev='xvdg' bus='xen'/> - </disk> - <controller type='usb' index='0'/> - <controller type='ide' index='0'/> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <memballoon model='none'/> - </devices> -</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3bfb5c4..094f1ea 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -752,7 +752,6 @@ mymain(void) QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); DO_TEST("disk-order", QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_VIRTIO_BLK_SCSI); - DO_TEST("disk-xenvbd", QEMU_CAPS_DRIVE_BOOT); DO_TEST("disk-drive-boot-disk", QEMU_CAPS_DRIVE_BOOT); DO_TEST("disk-drive-boot-cdrom", diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-xenvbd.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-xenvbd.xml deleted file mode 100644 index de6ca8b..0000000 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-xenvbd.xml +++ /dev/null @@ -1,51 +0,0 @@ -<domain type='qemu'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219136</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='i686' machine='pc'>hvm</type> - <boot dev='hd'/> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu</emulator> - <disk type='block' device='disk'> - <driver name='qemu' type='raw'/> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='ide'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> - <disk type='block' device='cdrom'> - <driver name='qemu' type='raw'/> - <source dev='/dev/HostVG/QEMUGuest2'/> - <target dev='hdc' bus='ide'/> - <readonly/> - <address type='drive' controller='0' bus='1' target='0' unit='0'/> - </disk> - <disk type='file' device='disk'> - <driver name='qemu' type='raw'/> - <source file='/tmp/data.img'/> - <target dev='xvda' bus='xen'/> - </disk> - <disk type='file' device='disk'> - <driver name='qemu' type='raw'/> - <source file='/tmp/logs.img'/> - <target dev='xvdg' bus='xen'/> - </disk> - <controller type='usb' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> - </controller> - <controller type='ide' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <memballoon model='none'/> - </devices> -</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 30cf3e8..ab03d1f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -365,7 +365,6 @@ mymain(void) DO_TEST("disk-cdrom-empty"); DO_TEST("disk-floppy"); DO_TEST("disk-many"); - DO_TEST("disk-xenvbd"); DO_TEST("disk-usb-device"); DO_TEST("disk-virtio"); DO_TEST("floppy-drive-fat"); -- 2.7.3
We no longer need to handle -usbdevice and the withDeviceArg logic becomes clearer. --- src/qemu/qemu_command.c | 30 ++++++------------------------ 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bb40c17..0bde505 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1905,23 +1905,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, unsigned int bootindex = 0; virDomainDiskDefPtr disk = def->disks[i]; bool withDeviceArg = false; - bool deviceFlagMasked = false; - - /* Unless we have -device, then USB disks need special - handling */ - if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - virCommandAddArg(cmd, "-usbdevice"); - virCommandAddArgFormat(cmd, "disk:%s", disk->src->path); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported usb disk type for '%s'"), - disk->src->path); - return -1; - } - continue; - } + bool deviceFlagMasked = true; /* PowerPC pseries based VMs do not support floppy device */ if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && @@ -1955,13 +1939,11 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, devices. Fortunately, those don't need static PCI addresses, so we don't really care that we can't use -device */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (disk->bus != VIR_DOMAIN_DISK_BUS_SD) { - withDeviceArg = true; - } else { - virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE); - deviceFlagMasked = true; - } + if (disk->bus != VIR_DOMAIN_DISK_BUS_SD) { + withDeviceArg = true; + } else { + virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE); + deviceFlagMasked = true; } optstr = qemuBuildDriveStr(disk, emitBootindex ? false : !!bootindex, -- 2.7.3
On 05/19/2016 02:59 PM, Ján Tomko wrote:
We no longer need to handle -usbdevice and the withDeviceArg logic becomes clearer. --- src/qemu/qemu_command.c | 30 ++++++------------------------ 1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bb40c17..0bde505 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1905,23 +1905,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, unsigned int bootindex = 0; virDomainDiskDefPtr disk = def->disks[i]; bool withDeviceArg = false; - bool deviceFlagMasked = false; - - /* Unless we have -device, then USB disks need special - handling */ - if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - virCommandAddArg(cmd, "-usbdevice"); - virCommandAddArgFormat(cmd, "disk:%s", disk->src->path); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported usb disk type for '%s'"), - disk->src->path); - return -1; - } - continue; - } + bool deviceFlagMasked = true;
I know this goes away by patch 7, but there's nothing later on that sets this to false - so it seems it should be kept as false for now... John
/* PowerPC pseries based VMs do not support floppy device */ if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && @@ -1955,13 +1939,11 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, devices. Fortunately, those don't need static PCI addresses, so we don't really care that we can't use -device */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (disk->bus != VIR_DOMAIN_DISK_BUS_SD) { - withDeviceArg = true; - } else { - virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE); - deviceFlagMasked = true; - } + if (disk->bus != VIR_DOMAIN_DISK_BUS_SD) { + withDeviceArg = true; + } else { + virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE); + deviceFlagMasked = true; } optstr = qemuBuildDriveStr(disk, emitBootindex ? false : !!bootindex,
Replace the two uses of the withDeviceArg bool in qemuBuildDiskDriveCommandLine and allow this function to be reused in qemuBuildDriveStr. --- src/qemu/qemu_command.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0bde505..a6cc0c9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1080,6 +1080,18 @@ qemuCheckFips(void) } +/* Unfortunately it is not possible to use + -device for floppies, or SD + devices. Fortunately, those don't need + static PCI addresses, so we don't really + care that we can't use -device */ +static bool +qemuDiskBusNeedsDeviceArg(int bus) +{ + return bus != VIR_DOMAIN_DISK_BUS_SD; +} + + char * qemuBuildDriveStr(virDomainDiskDefPtr disk, bool bootable, @@ -1904,7 +1916,6 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, char *optstr; unsigned int bootindex = 0; virDomainDiskDefPtr disk = def->disks[i]; - bool withDeviceArg = false; bool deviceFlagMasked = true; /* PowerPC pseries based VMs do not support floppy device */ @@ -1934,14 +1945,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, "-drive"); - /* Unfortunately it is not possible to use - -device for floppies, or SD - devices. Fortunately, those don't need - static PCI addresses, so we don't really - care that we can't use -device */ - if (disk->bus != VIR_DOMAIN_DISK_BUS_SD) { - withDeviceArg = true; - } else { + if (!qemuDiskBusNeedsDeviceArg(disk->bus)) { virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE); deviceFlagMasked = true; } @@ -1960,7 +1964,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, else if (disk->info.bootIndex) bootindex = disk->info.bootIndex; - if (withDeviceArg) { + if (qemuDiskBusNeedsDeviceArg(disk->bus)) { if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { if (virAsprintf(&optstr, "drive%c=drive-%s", disk->info.addr.drive.unit ? 'B' : 'A', -- 2.7.3
For some disk types (XEN, SD), we want to emit the syntax we used for disks before -device was available even if QEMU supports -device. Use the qemuDiskBusNeedsDeviceArg helper to figure out whether to use the old or new syntax. --- src/qemu/qemu_command.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a6cc0c9..55326c3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1106,6 +1106,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, char *source = NULL; int actualType = virStorageSourceGetActualType(disk->src); qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + bool emitDeviceSyntax = qemuDiskBusNeedsDeviceArg(disk->bus); if (idx < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1246,7 +1247,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } VIR_FREE(source); - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + if (emitDeviceSyntax) virBufferAddLit(&opt, "if=none"); else virBufferAsprintf(&opt, "if=%s", bus); @@ -1263,7 +1264,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } } - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (emitDeviceSyntax) { virBufferAsprintf(&opt, ",id=%s%s", QEMU_DRIVE_HOST_PREFIX, disk->info.alias); } else { if (busid == -1 && unitid == -1) { @@ -1916,7 +1917,6 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, char *optstr; unsigned int bootindex = 0; virDomainDiskDefPtr disk = def->disks[i]; - bool deviceFlagMasked = true; /* PowerPC pseries based VMs do not support floppy device */ if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && @@ -1945,15 +1945,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, "-drive"); - if (!qemuDiskBusNeedsDeviceArg(disk->bus)) { - virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE); - deviceFlagMasked = true; - } optstr = qemuBuildDriveStr(disk, emitBootindex ? false : !!bootindex, qemuCaps); - if (deviceFlagMasked) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE); if (!optstr) return -1; virCommandAddArg(cmd, optstr); -- 2.7.3
On 05/19/2016 02:59 PM, Ján Tomko wrote:
For some disk types (XEN, SD), we want to emit the syntax
Removed XEN in patch 4 I thought...
we used for disks before -device was available even if QEMU supports -device.
Use the qemuDiskBusNeedsDeviceArg helper to figure out whether to use the old or new syntax. --- src/qemu/qemu_command.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
The rest seems fine - although connecting the dots between CAPS_DEVICE and bus != BUS_SD wasn't as obvious... It's the double negative of qemuDiskBusNeedsDeviceArg to set deviceFlagMasked that caused me to pause. ACK series with a few touchups. John
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a6cc0c9..55326c3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1106,6 +1106,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, char *source = NULL; int actualType = virStorageSourceGetActualType(disk->src); qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + bool emitDeviceSyntax = qemuDiskBusNeedsDeviceArg(disk->bus);
if (idx < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1246,7 +1247,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } VIR_FREE(source);
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + if (emitDeviceSyntax) virBufferAddLit(&opt, "if=none"); else virBufferAsprintf(&opt, "if=%s", bus); @@ -1263,7 +1264,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } }
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (emitDeviceSyntax) { virBufferAsprintf(&opt, ",id=%s%s", QEMU_DRIVE_HOST_PREFIX, disk->info.alias); } else { if (busid == -1 && unitid == -1) { @@ -1916,7 +1917,6 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, char *optstr; unsigned int bootindex = 0; virDomainDiskDefPtr disk = def->disks[i]; - bool deviceFlagMasked = true;
/* PowerPC pseries based VMs do not support floppy device */ if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && @@ -1945,15 +1945,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
virCommandAddArg(cmd, "-drive");
- if (!qemuDiskBusNeedsDeviceArg(disk->bus)) { - virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE); - deviceFlagMasked = true; - } optstr = qemuBuildDriveStr(disk, emitBootindex ? false : !!bootindex, qemuCaps); - if (deviceFlagMasked) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE); if (!optstr) return -1; virCommandAddArg(cmd, optstr);
On Thu, May 19, 2016 at 05:29:41PM -0400, John Ferlan wrote:
On 05/19/2016 02:59 PM, Ján Tomko wrote:
For some disk types (XEN, SD), we want to emit the syntax
Removed XEN in patch 4 I thought...
we used for disks before -device was available even if QEMU supports -device.
Use the qemuDiskBusNeedsDeviceArg helper to figure out whether to use the old or new syntax. --- src/qemu/qemu_command.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
The rest seems fine - although connecting the dots between CAPS_DEVICE and bus != BUS_SD wasn't as obvious... It's the double negative of qemuDiskBusNeedsDeviceArg to set deviceFlagMasked that caused me to pause.
ACK series with a few touchups.
Thanks, pushed now. Jan
participants (2)
-
John Ferlan -
Ján Tomko