[libvirt] [PATCH 0/10]virtio-scsi: New device address logic for SCSI devices
This patch series completed the support for the first 3 parts of Paolo's proposal: http://permalink.gmane.org/gmane.comp.emulators.libvirt/50428 The 3 parts are: * SCSI controller models * Stable addressing for SCSI devices * LUN passthrough: block devices [PATCH 1/10] and [PATCH 2/10] add two new "scsi" controllers, "ibmvscsi" and "virtio-scsi". [PATCH 3/10] is to set the default controller model for scsi controller when parsing and implicit controller addtion. [PATCH 4/10] adds a helper functions to get a disk controller's model. [PATCH 5/10] introduces attribute "target" for device addressing XML. "target" is useless for model "lsilogic", for which it's not printed out when formating XML. [PATCH 7/10] builds the qemu command line for the new addressing format. The logic is: 1) If the disk controller model is "lsilogic": -drive file=/dev/sda,if=none,id=drive-scsi0-0-3,\ format=raw -device scsi-disk,bus=scsi0.0,\ scsi-id=0,drive=drive-scsi0-0-3-0,id=scsi0-0-3-0 libvirt attrs --> qdev properties: bus=scsi<controller>.0 scsi-id=<unit> 2) If the disk controller model is other else: The command line will be like: -drive file=/dev/sda,if=none,id=drive-scsi0-0-3-0,\ format=raw -device scsi-disk,bus=scsi0.0,channel=0,\ scsi-id=3,lun=0,drive=drive-scsi0-0-3-0,id=scsi0-0-3-0 libvirt attrs --> qdev properties: bus=scsi<controller>.0 channel=<bus> scsi-id=<target> lun=<unit> [PATCH 01/10] qemu: Add ibmvscsi controller model [PATCH 02/10] qemu: Add virtio-scsi controller model [PATCH 03/10] conf: Set default scsi controller model when parsing [PATCH 04/10] conf: Add helper function to look up disk controller [PATCH 05/10] conf: Introduce new attribute for device address [PATCH 06/10] qemu: New cap flag to indicate if channel is supported [PATCH 07/10] qemu: Build command line for the new address format [PATCH 08/10] tests: Update qemu tests to be consistent with new [PATCH 09/10] tests: Update vmx tests to be consistent with new [PATCH 10/10] tests: Add tests for virtio-scsi and ibmvscsi Regards, Osier
From: Paolo Bonzini <pbonzini@redhat.com> The codes to set default controller model is removed, it should be done when parsing in src/conf/domain_conf.c instead, setting default value for the command line and ignoring the domain def is not a good idea, an inactive domain can't known what the default setting should be, and thus you don't known how to give a right XML for it when formating. [PATCH 3/9] will introduce a new function to set the default value for controller model when parsing and adding controller implicitly. After this patch, the QEMU driver will actually look at the model and reject anything but auto, lsilogic and ibmvscsi. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Osier Yang <jyang@redhat.com> --- docs/formatdomain.html.in | 4 ++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 16 +++++++++++----- src/vmx/vmx.c | 9 +++++---- 6 files changed, 22 insertions(+), 12 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 5305f82..2dd3fd2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1657,8 +1657,8 @@ attributes <code>ports</code> and <code>vectors</code>, which control how many devices can be connected through the controller. A "scsi" controller has an optional - attribute <code>model</code>, which is one of "auto", - "buslogic", "lsilogic", "lsias1068", or "vmpvscsi". + attribute <code>model</code>, which is one of "auto", "buslogic", + "ibmvscsi", "lsilogic", "lsias1068" or "vmpvscsi". A "usb" controller has an optional attribute <code>model</code>, which is one of "piix3-uhci", "piix4-uhci", "ehci", "ich9-ehci1", "ich9-uhci1", "ich9-uhci2", "ich9-uhci3", diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index e276a92..d3deaea 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1120,6 +1120,7 @@ <value>lsilogic</value> <value>lsisas1068</value> <value>vmpvscsi</value> + <value>ibmvscsi</value> <value>piix3-uhci</value> <value>piix4-uhci</value> <value>ehci</value> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 85a2058..a23be05 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -238,7 +238,8 @@ VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAS "buslogic", "lsilogic", "lsisas1068", - "vmpvscsi") + "vmpvscsi", + "ibmvscsi") VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST, "piix3-uhci", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9c8792a..aa8c824 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -452,6 +452,7 @@ enum virDomainControllerModelSCSI { VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI, + VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5a34504..0cc552f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2350,7 +2350,7 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def, } char * -qemuBuildControllerDevStr(virDomainDefPtr domainDef, +qemuBuildControllerDevStr(virDomainDefPtr domainDef ATTRIBUTE_UNUSED, virDomainControllerDefPtr def, virBitmapPtr qemuCaps, int *nusbcontroller) @@ -2359,11 +2359,17 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: - if (STREQ(domainDef->os.arch, "ppc64") && - STREQ(domainDef->os.machine, "pseries")) { - virBufferAddLit(&buf, "spapr-vscsi"); - } else { + switch (def->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC: virBufferAddLit(&buf, "lsi"); + break; + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI: + virBufferAddLit(&buf, "spapr-vscsi"); + break; + default: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported controller model: %s"), + virDomainControllerModelSCSITypeToString(def->model)); } virBufferAsprintf(&buf, ",id=scsi%d", def->idx); break; diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 823d5df..5ebd81d 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -481,16 +481,17 @@ def->parallels[0]... #define VMX_BUILD_NAME(_suffix) \ VMX_BUILD_NAME_EXTRA(_suffix, #_suffix) -/* directly map the virDomainControllerModel to virVMXSCSIControllerModel, - * this is good enough for now because all virDomainControllerModel values - * are actually SCSI controller models in the ESX case */ +/* directly map the virDomainControllerModel to virVMXSCSIControllerModel. + * Using an uppercase name for unused values ensures that they will never + * be used. */ VIR_ENUM_DECL(virVMXControllerModelSCSI) VIR_ENUM_IMPL(virVMXControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, "auto", /* just to match virDomainControllerModel, will never be used */ "buslogic", "lsilogic", "lsisas1068", - "pvscsi"); + "pvscsi", + "UNUSED ibmvscsi"); -- 1.7.7.3
From: Paolo Bonzini <pbonzini@redhat.com> Adding a new model for virtio-scsi roughly follows the same scheme as the previous patch. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- --- docs/formatdomain.html.in | 2 +- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 3 +++ src/vmx/vmx.c | 3 ++- 6 files changed, 10 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 2dd3fd2..29497a0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1658,7 +1658,7 @@ control how many devices can be connected through the controller. A "scsi" controller has an optional attribute <code>model</code>, which is one of "auto", "buslogic", - "ibmvscsi", "lsilogic", "lsias1068" or "vmpvscsi". + "ibmvscsi", "lsilogic", "lsias1068", "virtio-scsi" or "vmpvscsi". A "usb" controller has an optional attribute <code>model</code>, which is one of "piix3-uhci", "piix4-uhci", "ehci", "ich9-ehci1", "ich9-uhci1", "ich9-uhci2", "ich9-uhci3", diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index d3deaea..724d7d0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1121,6 +1121,7 @@ <value>lsisas1068</value> <value>vmpvscsi</value> <value>ibmvscsi</value> + <value>virtio-scsi</value> <value>piix3-uhci</value> <value>piix4-uhci</value> <value>ehci</value> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a23be05..2da41f8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -239,7 +239,8 @@ VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAS "lsilogic", "lsisas1068", "vmpvscsi", - "ibmvscsi") + "ibmvscsi", + "virtio-scsi") VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST, "piix3-uhci", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index aa8c824..777bccb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -453,6 +453,7 @@ enum virDomainControllerModelSCSI { VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1068, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI, + VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0cc552f..ba22127 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2360,6 +2360,9 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef ATTRIBUTE_UNUSED, switch (def->type) { case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: switch (def->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: + virBufferAddLit(&buf, "virtio-scsi-pci"); + break; case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC: virBufferAddLit(&buf, "lsi"); break; diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 5ebd81d..1fdbd50 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -491,7 +491,8 @@ VIR_ENUM_IMPL(virVMXControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, "lsilogic", "lsisas1068", "pvscsi", - "UNUSED ibmvscsi"); + "UNUSED ibmvscsi", + "UNUSED virtio-scsi"); -- 1.7.7.3
And adding controller implicitly, ESX detects the default model automatically itself, and partly depends on the guest os. So we only try to set the default model for QEMU/KVM driver. --- src/conf/domain_conf.c | 32 ++++++++++++++++++++++++++++++-- 1 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2da41f8..ccdb80d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2502,6 +2502,28 @@ virDomainParseLegacyDeviceAddress(char *devaddr, return 0; } +static int +virDomainControllerDefaultModel(virDomainDefPtr def, + int type) +{ + switch(def->virtType) { + case VIR_DOMAIN_VIRT_TEST: + case VIR_DOMAIN_VIRT_QEMU: + case VIR_DOMAIN_VIRT_KQEMU: + case VIR_DOMAIN_VIRT_KVM: + if (type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { + if (STREQ(def->os.arch, "ppc64") && + STREQ(def->os.machine, "pseries")) { + return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; + } else { + return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; + } + } + default: + return -1; + } +} + int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) { @@ -3469,6 +3491,7 @@ virDomainControllerModelTypeFromString(const virDomainControllerDefPtr def, */ static virDomainControllerDefPtr virDomainControllerDefParseXML(xmlNodePtr node, + virDomainDefPtr dom, unsigned int flags) { virDomainControllerDefPtr def; @@ -3510,6 +3533,10 @@ virDomainControllerDefParseXML(xmlNodePtr node, def->model = -1; } + if (def->model == -1 || + def->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) + def->model = virDomainControllerDefaultModel(dom, def->type); + if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; @@ -6562,7 +6589,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; } else if (xmlStrEqual(node->name, BAD_CAST "controller")) { dev->type = VIR_DOMAIN_DEVICE_CONTROLLER; - if (!(dev->data.controller = virDomainControllerDefParseXML(node, flags))) + if (!(dev->data.controller = virDomainControllerDefParseXML(node, def, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "graphics")) { dev->type = VIR_DOMAIN_DEVICE_GRAPHICS; @@ -7124,7 +7151,7 @@ static int virDomainDefMaybeAddController(virDomainDefPtr def, cont->type = type; cont->idx = idx; - cont->model = -1; + cont->model = virDomainControllerDefaultModel(def, type); if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) { cont->opts.vioserial.ports = -1; @@ -7693,6 +7720,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto no_memory; for (i = 0 ; i < n ; i++) { virDomainControllerDefPtr controller = virDomainControllerDefParseXML(nodes[i], + def, flags); if (!controller) goto error; -- 1.7.7.3
On 02/17/2012 06:18 PM, Osier Yang wrote:
And adding controller implicitly, ESX detects the default model automatically itself, and partly depends on the guest os. So we only try to set the default model for QEMU/KVM driver. --- src/conf/domain_conf.c | 32 ++++++++++++++++++++++++++++++-- 1 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2da41f8..ccdb80d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2502,6 +2502,28 @@ virDomainParseLegacyDeviceAddress(char *devaddr, return 0; }
+static int +virDomainControllerDefaultModel(virDomainDefPtr def, + int type) +{ + switch(def->virtType) { + case VIR_DOMAIN_VIRT_TEST: + case VIR_DOMAIN_VIRT_QEMU: + case VIR_DOMAIN_VIRT_KQEMU: + case VIR_DOMAIN_VIRT_KVM: + if (type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { + if (STREQ(def->os.arch, "ppc64") && + STREQ(def->os.machine, "pseries")) { + return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; + } else { + return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; + } + } + default: + return -1; + } +} + int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) { @@ -3469,6 +3491,7 @@ virDomainControllerModelTypeFromString(const virDomainControllerDefPtr def, */ static virDomainControllerDefPtr virDomainControllerDefParseXML(xmlNodePtr node, + virDomainDefPtr dom, unsigned int flags) { virDomainControllerDefPtr def; @@ -3510,6 +3533,10 @@ virDomainControllerDefParseXML(xmlNodePtr node, def->model = -1; }
+ if (def->model == -1 || + def->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) + def->model = virDomainControllerDefaultModel(dom, def->type); +
I don't think this is correct. In fact, I think I misunderstood you while we discussed this before by email. So this patch is not necessary. Can you please resend the series without this patch, and with the test changes merged in the patch that affects the test output (presumably 4/10)? Paolo
if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error;
@@ -6562,7 +6589,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; } else if (xmlStrEqual(node->name, BAD_CAST "controller")) { dev->type = VIR_DOMAIN_DEVICE_CONTROLLER; - if (!(dev->data.controller = virDomainControllerDefParseXML(node, flags))) + if (!(dev->data.controller = virDomainControllerDefParseXML(node, def, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "graphics")) { dev->type = VIR_DOMAIN_DEVICE_GRAPHICS; @@ -7124,7 +7151,7 @@ static int virDomainDefMaybeAddController(virDomainDefPtr def,
cont->type = type; cont->idx = idx; - cont->model = -1; + cont->model = virDomainControllerDefaultModel(def, type);
if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) { cont->opts.vioserial.ports = -1; @@ -7693,6 +7720,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto no_memory; for (i = 0 ; i < n ; i++) { virDomainControllerDefPtr controller = virDomainControllerDefParseXML(nodes[i], + def, flags); if (!controller) goto error;
On 2012年02月18日 00:41, Paolo Bonzini wrote:
On 02/17/2012 06:18 PM, Osier Yang wrote:
And adding controller implicitly, ESX detects the default model automatically itself, and partly depends on the guest os. So we only try to set the default model for QEMU/KVM driver. --- src/conf/domain_conf.c | 32 ++++++++++++++++++++++++++++++-- 1 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2da41f8..ccdb80d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2502,6 +2502,28 @@ virDomainParseLegacyDeviceAddress(char *devaddr, return 0; }
+static int +virDomainControllerDefaultModel(virDomainDefPtr def, + int type) +{ + switch(def->virtType) { + case VIR_DOMAIN_VIRT_TEST: + case VIR_DOMAIN_VIRT_QEMU: + case VIR_DOMAIN_VIRT_KQEMU: + case VIR_DOMAIN_VIRT_KVM: + if (type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { + if (STREQ(def->os.arch, "ppc64")&& + STREQ(def->os.machine, "pseries")) { + return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; + } else { + return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; + } + } + default: + return -1; + } +} + int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) { @@ -3469,6 +3491,7 @@ virDomainControllerModelTypeFromString(const virDomainControllerDefPtr def, */ static virDomainControllerDefPtr virDomainControllerDefParseXML(xmlNodePtr node, + virDomainDefPtr dom, unsigned int flags) { virDomainControllerDefPtr def; @@ -3510,6 +3533,10 @@ virDomainControllerDefParseXML(xmlNodePtr node, def->model = -1; }
+ if (def->model == -1 || + def->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) + def->model = virDomainControllerDefaultModel(dom, def->type); +
I don't think this is correct. In fact, I think I misunderstood you while we discussed this before by email. So this patch is not necessary.
If you mean it's correct to check model in [PATCH 5/10] when formating the XML. This patch is neccessary, otherwise we have no way to known what model the driver sets by default, and thus no way to known how to format the XML. Even if we don't need to check the model when formating XML, I still think this patch is useful. Why we set default model silently, and don't show it up in XML? :-)
Can you please resend the series without this patch, and with the test changes merged in the patch that affects the test output (presumably 4/10)?
Paolo
if (virDomainDeviceInfoParseXML(node, NULL,&def->info, flags)< 0) goto error;
@@ -6562,7 +6589,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, goto error; } else if (xmlStrEqual(node->name, BAD_CAST "controller")) { dev->type = VIR_DOMAIN_DEVICE_CONTROLLER; - if (!(dev->data.controller = virDomainControllerDefParseXML(node, flags))) + if (!(dev->data.controller = virDomainControllerDefParseXML(node, def, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "graphics")) { dev->type = VIR_DOMAIN_DEVICE_GRAPHICS; @@ -7124,7 +7151,7 @@ static int virDomainDefMaybeAddController(virDomainDefPtr def,
cont->type = type; cont->idx = idx; - cont->model = -1; + cont->model = virDomainControllerDefaultModel(def, type);
if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) { cont->opts.vioserial.ports = -1; @@ -7693,6 +7720,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto no_memory; for (i = 0 ; i< n ; i++) { virDomainControllerDefPtr controller = virDomainControllerDefParseXML(nodes[i], + def, flags); if (!controller) goto error;
On 02/17/2012 05:58 PM, Osier Yang wrote:
I don't think this is correct. In fact, I think I misunderstood you while we discussed this before by email. So this patch is not necessary.
If you mean it's correct to check model in [PATCH 5/10] when formating the XML. This patch is neccessary, otherwise we have no way to known what model the driver sets by default, and thus no way to known how to format the XML.
Right, but we should always print "target".
Even if we don't need to check the model when formating XML, I still think this patch is useful. Why we set default model silently, and don't show it up in XML? :-)
The default is "auto", no? Which means spapr-vscsi on ppc64, and lsi otherwise (unfortunately, but that's life). Paolo
--- src/conf/domain_conf.c | 17 +++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 21 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ccdb80d..c36ac18 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2525,6 +2525,23 @@ virDomainControllerDefaultModel(virDomainDefPtr def, } int +virDomainDiskFindControllerModel(virDomainDefPtr def, + virDomainDiskDefPtr disk, + int controllerType) +{ + int model = -1; + int i; + + for (i = 0; i < def->ncontrollers; i++) { + if (def->controllers[i]->type == controllerType && + def->controllers[i]->idx == disk->info.addr.drive.controller) + model = def->controllers[i]->model; + } + + return model; +} + +int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) { int idx = virDiskNameToIndex(def->dst); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 777bccb..fed7cc3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1729,6 +1729,9 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def); void virDomainInputDefFree(virDomainInputDefPtr def); void virDomainDiskDefFree(virDomainDiskDefPtr def); void virDomainDiskHostDefFree(virDomainDiskHostDefPtr def); +int virDomainDiskFindControllerModel(virDomainDefPtr def, + virDomainDiskDefPtr disk, + int controllerType); void virDomainControllerDefFree(virDomainControllerDefPtr def); void virDomainFSDefFree(virDomainFSDefPtr def); void virDomainActualNetDefFree(virDomainActualNetDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9e3573f..29a8da5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -302,6 +302,7 @@ virDomainDiskDefFree; virDomainDiskDeviceTypeToString; virDomainDiskErrorPolicyTypeFromString; virDomainDiskErrorPolicyTypeToString; +virDomainDiskFindControllerModel; virDomainDiskIndexByName; virDomainDiskInsert; virDomainDiskInsertPreAlloced; -- 1.7.7.3
* src/conf/domain_conf.h: Add new member "target" to struct _virDomainDeviceDriveAddress. * src/conf/domain_conf.c: - virDomainDeviceDriveAddressParseXML: Parse "target". - virDomainDeviceInfoFormat: new parameter "controllerType" (disk controller type) and "bus" (disk bus type)" to distiguish the lsilogic model and other models of disk controller, if the model is lsilogic, then old address format (without "target") is printed, otherwise new address format (with "target") is printed. - virDomainDiskDefFormat: New parameter "vmdef", to pass to virDomainDiskFindControllerModel as an argument. - New helper function virDomainDeviceInfoFormatCommon, which is an simple wrapper of virDomainDeviceInfoFormat. - Replace virDomainDeviceInfoFormat with virDomainDeviceInfoFormatCommon accross the file, except the one used for disk def formating. * docs/schemas/domaincommon.rng: Define the rng schema for "target". * docs/formatdomain.html.in: Document "target". --- docs/formatdomain.html.in | 13 +++++- docs/schemas/domaincommon.rng | 10 +++++ src/conf/domain_conf.c | 87 ++++++++++++++++++++++++++++------------ src/conf/domain_conf.h | 1 + 4 files changed, 83 insertions(+), 28 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 29497a0..f923c75 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1094,7 +1094,14 @@ <target dev='hdc' bus='ide'/> <readonly/> </disk> + <disk type='block' device='lun'> + <driver name='qemu' type='raw'/> + <source dev='/dev/sda'/< + <target dev='sda' bus='scsi'/< + <address type='drive' controller='0' bus='0' target='3' unit='0'/< + </disk> </devices> + ...</pre> <dl> @@ -1401,8 +1408,9 @@ Multifunction defaults to 'off'; any other value requires QEMU 0.1.3 and <span class="since">libvirt 0.9.7</span>. For a "drive" controller, additional attributes - <code>controller</code>, <code>bus</code>, - and <code>unit</code> are available, each defaulting to 0. + <code>controller</code>, <code>bus</code>, <code>target</code> + (<span class="since">libvirt 0.9.11</span>), and <code>unit</code> + are available, each defaulting to 0. </dd> <dt><code>auth</code></dt> <dd>If present, the <code>auth</code> element provides the @@ -1592,6 +1600,7 @@ <dd>Drive addresses have the following additional attributes: <code>controller</code> (a 2-digit controller number), <code>bus</code> (a 2-digit bus number), + <code>target</code> (a 2-digit bus number), and <code>unit</code> (a 2-digit unit number on the bus). </dd> <dt><code>type='virtio-serial'</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 724d7d0..3908733 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2460,6 +2460,11 @@ </attribute> </optional> <optional> + <attribute name="target"> + <ref name="driveTarget"/> + </attribute> + </optional> + <optional> <attribute name="unit"> <ref name="driveUnit"/> </attribute> @@ -3147,6 +3152,11 @@ <param name="pattern">[0-9]{1,2}</param> </data> </define> + <define name="driveTarget"> + <data type="string"> + <param name="pattern">[0-9]{1,2}</param> + </data> + </define> <define name="driveUnit"> <data type="string"> <param name="pattern">[0-9]{1,2}</param> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c36ac18..b8e5a78 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -240,7 +240,7 @@ VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAS "lsisas1068", "vmpvscsi", "ibmvscsi", - "virtio-scsi") + "virtio-scsi"); VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST, "piix3-uhci", @@ -1913,11 +1913,13 @@ void virDomainDefClearDeviceAliases(virDomainDefPtr def) /* Generate a string representation of a device address - * @param address Device address to stringify + * @info address Device address to stringify */ static int ATTRIBUTE_NONNULL(2) virDomainDeviceInfoFormat(virBufferPtr buf, virDomainDeviceInfoPtr info, + int controllerModel, + int bus, unsigned int flags) { if ((flags & VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) && info->bootIndex) @@ -1975,10 +1977,19 @@ virDomainDeviceInfoFormat(virBufferPtr buf, break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE: - virBufferAsprintf(buf, " controller='%d' bus='%d' unit='%d'", - info->addr.drive.controller, - info->addr.drive.bus, - info->addr.drive.unit); + if ((bus == VIR_DOMAIN_DISK_BUS_SCSI) && + (controllerModel != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC)) { + virBufferAsprintf(buf, " controller='%d' bus='%d' target='%d' unit='%d'", + info->addr.drive.controller, + info->addr.drive.bus, + info->addr.drive.target, + info->addr.drive.unit); + } else { + virBufferAsprintf(buf, " controller='%d' bus='%d' unit='%d'", + info->addr.drive.controller, + info->addr.drive.bus, + info->addr.drive.unit); + } break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL: @@ -2016,6 +2027,12 @@ virDomainDeviceInfoFormat(virBufferPtr buf, return 0; } +static int ATTRIBUTE_NONNULL(2) +virDomainDeviceInfoFormatCommon(virBufferPtr buf, + virDomainDeviceInfoPtr info, + unsigned int flags) { + return virDomainDeviceInfoFormat(buf, info, -1, -1, flags); +} static int virDomainDevicePCIAddressParseXML(xmlNodePtr node, @@ -2090,13 +2107,14 @@ static int virDomainDeviceDriveAddressParseXML(xmlNodePtr node, virDomainDeviceDriveAddressPtr addr) { - char *bus, *unit, *controller; + char *bus, *unit, *controller, *target; int ret = -1; memset(addr, 0, sizeof(*addr)); controller = virXMLPropString(node, "controller"); bus = virXMLPropString(node, "bus"); + target = virXMLPropString(node, "target"); unit = virXMLPropString(node, "unit"); if (controller && @@ -2113,6 +2131,13 @@ virDomainDeviceDriveAddressParseXML(xmlNodePtr node, goto cleanup; } + if (target && + virStrToLong_ui(target, NULL, 10, &addr->target) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'target' attribute")); + goto cleanup; + } + if (unit && virStrToLong_ui(unit, NULL, 10, &addr->unit) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -2125,6 +2150,7 @@ virDomainDeviceDriveAddressParseXML(xmlNodePtr node, cleanup: VIR_FREE(controller); VIR_FREE(bus); + VIR_FREE(target); VIR_FREE(unit); return ret; } @@ -3550,6 +3576,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, def->model = -1; } + /* ESX detects the model itself if model is "auto". */ if (def->model == -1 || def->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) def->model = virDomainControllerDefaultModel(dom, def->type); @@ -10072,6 +10099,7 @@ virDomainLeaseDefFormat(virBufferPtr buf, static int virDomainDiskDefFormat(virBufferPtr buf, + virDomainDefPtr vmdef, virDomainDiskDefPtr def, unsigned int flags) { @@ -10088,6 +10116,7 @@ virDomainDiskDefFormat(virBufferPtr buf, const char *startupPolicy = virDomainStartupPolicyTypeToString(def->startupPolicy); char uuidstr[VIR_UUID_STRING_BUFLEN]; + int controllerModel = -1; if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -10297,7 +10326,13 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAdjustIndent(buf, -6); } - if (virDomainDeviceInfoFormat(buf, &def->info, + if (def->bus == VIR_DOMAIN_DISK_BUS_SCSI) { + controllerModel = + virDomainDiskFindControllerModel(vmdef, def, + VIR_DOMAIN_CONTROLLER_TYPE_SCSI); + } + + if (virDomainDeviceInfoFormat(buf, &def->info, controllerModel, def->bus, flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT) < 0) return -1; @@ -10368,7 +10403,7 @@ virDomainControllerDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, flags) < 0) return -1; virBufferAddLit(buf, " </controller>\n"); } else { @@ -10445,7 +10480,7 @@ virDomainFSDefFormat(virBufferPtr buf, if (def->readonly) virBufferAddLit(buf, " <readonly/>\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, flags) < 0) return -1; virBufferAddLit(buf, " </filesystem>\n"); @@ -10676,9 +10711,9 @@ virDomainNetDefFormat(virBufferPtr buf, return -1; virBufferAdjustIndent(buf, -6); - if (virDomainDeviceInfoFormat(buf, &def->info, - flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT - | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, + flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT + | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) < 0) return -1; virBufferAddLit(buf, " </interface>\n"); @@ -10870,7 +10905,7 @@ virDomainChrDefFormat(virBufferPtr buf, } if (virDomainDeviceInfoIsSet(&def->info, flags)) { - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, flags) < 0) return -1; } @@ -10923,7 +10958,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf, _("unexpected smartcard type %d"), def->type); return -1; } - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, flags) < 0) return -1; virBufferAddLit(buf, " </smartcard>\n"); return 0; @@ -10946,7 +10981,7 @@ virDomainSoundDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, flags) < 0) return -1; virBufferAddLit(buf, " </sound>\n"); } else { @@ -10974,7 +11009,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, flags) < 0) return -1; virBufferAddLit(buf, " </memballoon>\n"); } else { @@ -11021,7 +11056,7 @@ virDomainWatchdogDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, flags) < 0) return -1; virBufferAddLit(buf, " </watchdog>\n"); } else { @@ -11072,7 +11107,7 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, flags) < 0) return -1; virBufferAddLit(buf, " </video>\n"); @@ -11104,7 +11139,7 @@ virDomainInputDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, flags) < 0) return -1; virBufferAddLit(buf, " </input>\n"); } else { @@ -11510,9 +11545,9 @@ virDomainHostdevDefFormat(virBufferPtr buf, virBufferAddLit(buf, " </source>\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, - flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT - | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, + flags | VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT + | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) < 0) return -1; virBufferAddLit(buf, " </hostdev>\n"); @@ -11532,7 +11567,7 @@ virDomainRedirdevDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <redirdev bus='%s'", bus); if (virDomainChrSourceDefFormat(buf, &def->source.chr, false, flags) < 0) return -1; - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, flags) < 0) return -1; virBufferAddLit(buf, " </redirdev>\n"); @@ -11556,7 +11591,7 @@ virDomainHubDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormatCommon(buf, &def->info, flags) < 0) return -1; virBufferAddLit(buf, " </hub>\n"); } else { @@ -11937,7 +11972,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->emulator); for (n = 0 ; n < def->ndisks ; n++) - if (virDomainDiskDefFormat(buf, def->disks[n], flags) < 0) + if (virDomainDiskDefFormat(buf, def, def->disks[n], flags) < 0) goto cleanup; for (n = 0 ; n < def->ncontrollers ; n++) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fed7cc3..596be4d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -106,6 +106,7 @@ typedef virDomainDeviceDriveAddress *virDomainDeviceDriveAddressPtr; struct _virDomainDeviceDriveAddress { unsigned int controller; unsigned int bus; + unsigned int target; unsigned int unit; }; -- 1.7.7.3
On 02/17/2012 06:18 PM, Osier Yang wrote:
+ if ((bus == VIR_DOMAIN_DISK_BUS_SCSI) && + (controllerModel != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC)) { + virBufferAsprintf(buf, " controller='%d' bus='%d' target='%d' unit='%d'", + info->addr.drive.controller, + info->addr.drive.bus, + info->addr.drive.target, + info->addr.drive.unit); + } else { + virBufferAsprintf(buf, " controller='%d' bus='%d' unit='%d'", + info->addr.drive.controller, + info->addr.drive.bus, + info->addr.drive.unit); + }
Yeah, I definitely misunderstood. Here it is correct to print the target for all models. Paolo
On 2012年02月18日 00:41, Paolo Bonzini wrote:
On 02/17/2012 06:18 PM, Osier Yang wrote:
+ if ((bus == VIR_DOMAIN_DISK_BUS_SCSI)&& + (controllerModel != VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC)) { + virBufferAsprintf(buf, " controller='%d' bus='%d' target='%d' unit='%d'", + info->addr.drive.controller, + info->addr.drive.bus, + info->addr.drive.target, + info->addr.drive.unit); + } else { + virBufferAsprintf(buf, " controller='%d' bus='%d' unit='%d'", + info->addr.drive.controller, + info->addr.drive.bus, + info->addr.drive.unit); + }
Yeah, I definitely misunderstood. Here it is correct to print the target for all models.
Do you mean it's correct check the model here? I think the "lsilogic" model won't want to see the "target". It's complete useless. Osier
On 02/17/2012 05:54 PM, Osier Yang wrote:
Yeah, I definitely misunderstood. Here it is correct to print the target for all models.
Do you mean it's correct check the model here? I think the "lsilogic" model won't want to see the "target". It's complete useless.
It's true that it's useless. However, there is no reason not to print it; so is "bus", for example. Also, even for non-SCSI disks you need to print the target. You may omit it if it is zero, but I'm not sure it brings any benefit to do so. By the way, you need to test for "target == 0" and print an error also when creating an IDE or AHCI disk. Paolo
On 2012年02月18日 01:03, Paolo Bonzini wrote:
On 02/17/2012 05:54 PM, Osier Yang wrote:
Yeah, I definitely misunderstood. Here it is correct to print the target for all models.
Do you mean it's correct check the model here? I think the "lsilogic" model won't want to see the "target". It's complete useless.
It's true that it's useless. However, there is no reason not to print it; so is "bus", for example. Also, even for non-SCSI disks you need to print the target. You may omit it if it is zero, but I'm not sure it brings any benefit to do so.
Finally I understand you, yeah, it's no harm to print the target. Except we need another patch to change lots of the tests.
By the way, you need to test for "target == 0" and print an error also when creating an IDE or AHCI disk.
Agreed.
Paolo
--- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6d35676..64a4546 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -153,6 +153,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "drive-iotune", /* 85 */ "system_wakeup", + "scsi-disk.channel", ); struct qemu_feature_flags { @@ -1363,6 +1364,7 @@ qemuCapsExtractDeviceStr(const char *qemu, "-device", "pci-assign,?", "-device", "virtio-blk-pci,?", "-device", "virtio-net-pci,?", + "-device", "scsi-disk,?", NULL); virCommandAddEnvPassCommon(cmd); /* qemu -help goes to stdout, but qemu -device ? goes to stderr. */ @@ -1440,6 +1442,8 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr flags) qemuCapsSet(flags, QEMU_CAPS_VIRTIO_NET_EVENT_IDX); if (strstr(str, "virtio-blk-pci.scsi")) qemuCapsSet(flags, QEMU_CAPS_VIRTIO_BLK_SCSI); + if (strstr(str, "scsi-disk.channel")) + qemuCapsSet(flags, QEMU_CAPS_SCSI_DISK_CHANNEL); return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b9666e1..db584ce 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -121,6 +121,7 @@ enum qemuCapsFlags { QEMU_CAPS_FSDEV_WRITEOUT = 84, /* -fsdev writeout supported */ QEMU_CAPS_DRIVE_IOTUNE = 85, /* -drive bps= and friends */ QEMU_CAPS_WAKEUP = 86, /* system_wakeup monitor command */ + QEMU_CAPS_SCSI_DISK_CHANNEL = 87, /* Is scsi-disk.channel available? */ QEMU_CAPS_LAST, /* this must always be the last item */ }; -- 1.7.7.3
For any disk controller model which is not "lsilogic", the command line will be like: -drive file=/dev/sda,if=none,id=drive-scsi0-0-3-0,format=raw \ -device scsi-disk,bus=scsi0.0,channel=0,scsi-id=3,lun=0,i\ drive=drive-scsi0-0-3-0,id=scsi0-0-3-0 The relationship between the libvirt address attrs and the qdev properties are (controller model is not "lsilogic"; strings inside <> represent libvirt adress attrs): bus=scsi<controller>.0 channel=<bus> scsi-id=<target> lun=<unit> * src/qemu/qemu_command.h: (New param "virDomainDefPtr def" for function qemuBuildDriveDevStr; new param "virDomainDefPtr vmdef" for function qemuAssignDeviceDiskAlias. Both for virDomainDiskFindControllerModel's use). * src/qemu/qemu_command.c: - New param "virDomainDefPtr def" for qemuAssignDeviceDiskAliasCustom. For virDomainDiskFindControllerModel's use, if the disk bus is "scsi" and the controller model is not "lsilogic", "target" is one part of the alias name. - According change on qemuAssignDeviceDiskAlias and qemuBuildDriveDevStr * src/qemu/qemu_hotplug.c: - Changes to be consistent with declarations of qemuAssignDeviceDiskAlias qemuBuildDriveDevStr, and qemuBuildControllerDevStr. --- src/qemu/qemu_command.c | 96 ++++++++++++++++++++++++++++++++++++---------- src/qemu/qemu_command.h | 10 +++-- src/qemu/qemu_hotplug.c | 14 +++--- 3 files changed, 88 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba22127..a964a9b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -463,15 +463,35 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk) /* Our custom -drive naming scheme used with id= */ -static int qemuAssignDeviceDiskAliasCustom(virDomainDiskDefPtr disk) +static int +qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def, + virDomainDiskDefPtr disk) { const char *prefix = virDomainDiskBusTypeToString(disk->bus); + int controllerModel = -1; + if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { - if (virAsprintf(&disk->info.alias, "%s%d-%d-%d", prefix, - disk->info.addr.drive.controller, - disk->info.addr.drive.bus, - disk->info.addr.drive.unit) < 0) - goto no_memory; + if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) { + controllerModel = + virDomainDiskFindControllerModel(def, disk, + VIR_DOMAIN_CONTROLLER_TYPE_SCSI); + } + + if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI || + controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { + if (virAsprintf(&disk->info.alias, "%s%d-%d-%d", prefix, + disk->info.addr.drive.controller, + disk->info.addr.drive.bus, + disk->info.addr.drive.unit) < 0) + goto no_memory; + } else { + if (virAsprintf(&disk->info.alias, "%s%d-%d-%d-%d", prefix, + disk->info.addr.drive.controller, + disk->info.addr.drive.bus, + disk->info.addr.drive.target, + disk->info.addr.drive.unit) < 0) + goto no_memory; + } } else { int idx = virDiskNameToIndex(disk->dst); if (virAsprintf(&disk->info.alias, "%s-disk%d", prefix, idx) < 0) @@ -487,11 +507,13 @@ no_memory: int -qemuAssignDeviceDiskAlias(virDomainDiskDefPtr def, virBitmapPtr qemuCaps) +qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef, + virDomainDiskDefPtr def, + virBitmapPtr qemuCaps) { if (qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) { if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - return qemuAssignDeviceDiskAliasCustom(def); + return qemuAssignDeviceDiskAliasCustom(vmdef, def); else return qemuAssignDeviceDiskAliasFixed(def); } else { @@ -602,7 +624,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virBitmapPtr qemuCaps) int i; for (i = 0; i < def->ndisks ; i++) { - if (qemuAssignDeviceDiskAlias(def->disks[i], qemuCaps) < 0) + if (qemuAssignDeviceDiskAlias(def, def->disks[i], qemuCaps) < 0) return -1; } if (qemuCapsGet(qemuCaps, QEMU_CAPS_NET_NAME) || @@ -2078,13 +2100,15 @@ error: char * -qemuBuildDriveDevStr(virDomainDiskDefPtr disk, +qemuBuildDriveDevStr(virDomainDefPtr def, + virDomainDiskDefPtr disk, int bootindex, virBitmapPtr qemuCaps) { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); int idx = virDiskNameToIndex(disk->dst); + int controller_model; if (idx < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -2096,7 +2120,8 @@ qemuBuildDriveDevStr(virDomainDiskDefPtr disk, /* make sure that both the bus and the qemu binary support * type='lun' (SG_IO). */ - if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && + disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk device='lun' is not supported for bus='%s'"), bus); @@ -2123,11 +2148,40 @@ qemuBuildDriveDevStr(virDomainDiskDefPtr disk, disk->info.addr.drive.unit); break; case VIR_DOMAIN_DISK_BUS_SCSI: - virBufferAddLit(&opt, "scsi-disk"); - virBufferAsprintf(&opt, ",bus=scsi%d.%d,scsi-id=%d", - disk->info.addr.drive.controller, - disk->info.addr.drive.bus, - disk->info.addr.drive.unit); + controller_model = + virDomainDiskFindControllerModel(def, disk, + VIR_DOMAIN_CONTROLLER_TYPE_SCSI); + if (controller_model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { + virBufferAddLit(&opt, "scsi-disk"); + virBufferAsprintf(&opt, ",bus=scsi%d.%d,scsi-id=%d", + disk->info.addr.drive.controller, + disk->info.addr.drive.bus, + disk->info.addr.drive.unit); + } else { + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { + if (disk->info.addr.drive.target > 7) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support target " + "greater than 7")); + goto error; + } + + if ((disk->info.addr.drive.bus != disk->info.addr.drive.unit) && + (disk->info.addr.drive.bus != 0)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU only supports both bus and " + "unit are equal to 0")); + goto error; + } + } + + virBufferAddLit(&opt, "scsi-disk"); + virBufferAsprintf(&opt, ",bus=scsi%d.0,channel=%d,scsi-id=%d,lun=%d", + disk->info.addr.drive.controller, + disk->info.addr.drive.bus, + disk->info.addr.drive.target, + disk->info.addr.drive.unit); + } break; case VIR_DOMAIN_DISK_BUS_SATA: virBufferAddLit(&opt, "ide-drive"); @@ -2323,6 +2377,7 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def, int model, caps; model = def->model; + if (model == -1) model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI; @@ -2350,8 +2405,7 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def, } char * -qemuBuildControllerDevStr(virDomainDefPtr domainDef ATTRIBUTE_UNUSED, - virDomainControllerDefPtr def, +qemuBuildControllerDevStr(virDomainControllerDefPtr def, virBitmapPtr qemuCaps, int *nusbcontroller) { @@ -4339,7 +4393,7 @@ qemuBuildCommandLine(virConnectPtr conn, char *devstr; virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildControllerDevStr(def, cont, qemuCaps, NULL))) + if (!(devstr = qemuBuildControllerDevStr(cont, qemuCaps, NULL))) goto error; virCommandAddArg(cmd, devstr); @@ -4358,7 +4412,7 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-device"); char *devstr; - if (!(devstr = qemuBuildControllerDevStr(def, def->controllers[i], qemuCaps, + if (!(devstr = qemuBuildControllerDevStr(def->controllers[i], qemuCaps, &usbcontroller))) goto error; @@ -4477,7 +4531,7 @@ qemuBuildCommandLine(virConnectPtr conn, } else { virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildDriveDevStr(disk, bootindex, + if (!(optstr = qemuBuildDriveDevStr(def, disk, bootindex, qemuCaps))) goto error; virCommandAddArg(cmd, optstr); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 2f8b5ba..4e2f5c0 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -90,14 +90,14 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, virBitmapPtr qemuCaps); /* Current, best practice */ -char * qemuBuildDriveDevStr(virDomainDiskDefPtr disk, +char * qemuBuildDriveDevStr(virDomainDefPtr def, + virDomainDiskDefPtr disk, int bootindex, virBitmapPtr qemuCaps); char * qemuBuildFSDevStr(virDomainFSDefPtr fs, virBitmapPtr qemuCaps); /* Current, best practice */ -char * qemuBuildControllerDevStr(virDomainDefPtr domainDef, - virDomainControllerDefPtr def, +char * qemuBuildControllerDevStr(virDomainControllerDefPtr def, virBitmapPtr qemuCaps, int *nusbcontroller); @@ -201,7 +201,9 @@ int qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr ad int qemuAssignDeviceAliases(virDomainDefPtr def, virBitmapPtr qemuCaps); int qemuDomainNetVLAN(virDomainNetDefPtr def); int qemuAssignDeviceNetAlias(virDomainDefPtr def, virDomainNetDefPtr net, int idx); -int qemuAssignDeviceDiskAlias(virDomainDiskDefPtr def, virBitmapPtr qemuCaps); +int qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef, + virDomainDiskDefPtr def, + virBitmapPtr qemuCaps); int qemuAssignDeviceHostdevAlias(virDomainDefPtr def, virDomainHostdevDefPtr hostdev, int idx); int qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller); int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr redirdev, int idx); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3dd7c0a..d7842c6 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -226,13 +226,13 @@ int qemuDomainAttachPciDiskDevice(virConnectPtr conn, if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0) goto error; releaseaddr = true; - if (qemuAssignDeviceDiskAlias(disk, priv->qemuCaps) < 0) + if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error; if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps))) goto error; - if (!(devstr = qemuBuildDriveDevStr(disk, 0, priv->qemuCaps))) + if (!(devstr = qemuBuildDriveDevStr(NULL, disk, 0, priv->qemuCaps))) goto error; } @@ -336,7 +336,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver, goto cleanup; } - if (!(devstr = qemuBuildControllerDevStr(vm->def, controller, priv->qemuCaps, NULL))) { + if (!(devstr = qemuBuildControllerDevStr(controller, priv->qemuCaps, NULL))) { goto cleanup; } } @@ -461,9 +461,9 @@ int qemuDomainAttachSCSIDisk(virConnectPtr conn, } if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuAssignDeviceDiskAlias(disk, priv->qemuCaps) < 0) + if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error; - if (!(devstr = qemuBuildDriveDevStr(disk, 0, priv->qemuCaps))) + if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) goto error; } @@ -583,11 +583,11 @@ int qemuDomainAttachUsbMassstorageDevice(virConnectPtr conn, } if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuAssignDeviceDiskAlias(disk, priv->qemuCaps) < 0) + if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error; if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps))) goto error; - if (!(devstr = qemuBuildDriveDevStr(disk, 0, priv->qemuCaps))) + if (!(devstr = qemuBuildDriveDevStr(NULL, disk, 0, priv->qemuCaps))) goto error; } -- 1.7.7.3
On 02/17/2012 06:18 PM, Osier Yang wrote:
+ controller_model = + virDomainDiskFindControllerModel(def, disk, + VIR_DOMAIN_CONTROLLER_TYPE_SCSI); + if (controller_model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { + virBufferAddLit(&opt, "scsi-disk"); + virBufferAsprintf(&opt, ",bus=scsi%d.%d,scsi-id=%d", + disk->info.addr.drive.controller, + disk->info.addr.drive.bus, + disk->info.addr.drive.unit);
Here indeed it is ok to look at the model, for backwards compatibility. However, you need to test that the target is zero and print an error if it is not (for example, "legacy addressing for the 'lsilogic' SCSI controller does not use the target attribute"). Paolo
+ } else { + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { + if (disk->info.addr.drive.target > 7) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support target " + "greater than 7")); + goto error; + } + + if ((disk->info.addr.drive.bus != disk->info.addr.drive.unit) && + (disk->info.addr.drive.bus != 0)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU only supports both bus and " + "unit are equal to 0")); + goto error; + } + } + + virBufferAddLit(&opt, "scsi-disk"); + virBufferAsprintf(&opt, ",bus=scsi%d.0,channel=%d,scsi-id=%d,lun=%d", + disk->info.addr.drive.controller, + disk->info.addr.drive.bus, + disk->info.addr.drive.target, + disk->info.addr.drive.unit); + }
On 2012年02月18日 00:44, Paolo Bonzini wrote:
On 02/17/2012 06:18 PM, Osier Yang wrote:
+ controller_model = + virDomainDiskFindControllerModel(def, disk, + VIR_DOMAIN_CONTROLLER_TYPE_SCSI); + if (controller_model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { + virBufferAddLit(&opt, "scsi-disk"); + virBufferAsprintf(&opt, ",bus=scsi%d.%d,scsi-id=%d", + disk->info.addr.drive.controller, + disk->info.addr.drive.bus, + disk->info.addr.drive.unit);
Here indeed it is ok to look at the model, for backwards compatibility. However, you need to test that the target is zero and print an error if it is not (for example, "legacy addressing for the 'lsilogic' SCSI controller does not use the target attribute").
Yeah, agreed. Thanks for pointing it out.
Paolo
+ } else { + if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_CHANNEL)) { + if (disk->info.addr.drive.target> 7) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support target " + "greater than 7")); + goto error; + } + + if ((disk->info.addr.drive.bus != disk->info.addr.drive.unit)&& + (disk->info.addr.drive.bus != 0)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU only supports both bus and " + "unit are equal to 0")); + goto error; + } + } + + virBufferAddLit(&opt, "scsi-disk"); + virBufferAsprintf(&opt, ",bus=scsi%d.0,channel=%d,scsi-id=%d,lun=%d", + disk->info.addr.drive.controller, + disk->info.addr.drive.bus, + disk->info.addr.drive.target, + disk->info.addr.drive.unit); + }
--- .../qemuxml2argv-disk-scsi-device.xml | 2 +- .../qemuxml2argv-pseries-vio-user-assigned.args | 10 +++++++++- .../qemuxml2argvdata/qemuxml2argv-pseries-vio.args | 10 +++++++++- .../qemuxml2xmlout-disk-scsi-device-auto.xml | 2 +- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-device.xml index 17b59d8..9456335 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-device.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-device.xml @@ -26,7 +26,7 @@ </disk> <controller type='usb' index='0'/> <controller type='ide' index='0'/> - <controller type='scsi' index='0'/> + <controller type='scsi' index='0' model='lsilogic'/> <memballoon model='virtio'/> </devices> </domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.args index e939e1b..aed9ca8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.args @@ -1 +1,9 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-ppc64 -S -M pseries -m 512 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device spapr-vscsi,id=scsi0,reg=0x2000 -device spapr-vscsi,id=scsi1,reg=0x30000000 -drive file=/tmp/scsidisk.img,if=none,id=drive-scsi1-0-0 -device scsi-disk,bus=scsi1.0,scsi-id=0,drive=drive-scsi1-0-0,id=scsi1-0-0 -chardev pty,id=charserial0 -device spapr-vty,chardev=charserial0,reg=0x20000000 -chardev pty,id=charserial1 -device spapr-vty,chardev=charserial1,reg=0x30001000 -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-ppc64 \ +-S -M pseries -m 512 -smp 1 -nographic -nodefconfig -nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ +-device spapr-vscsi,id=scsi0,reg=0x2000 -device spapr-vscsi,id=scsi1,reg=0x30000000 \ +-drive file=/tmp/scsidisk.img,if=none,id=drive-scsi1-0-0-0 \ +-device scsi-disk,bus=scsi1.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi1-0-0-0,id=scsi1-0-0-0 \ +-chardev pty,id=charserial0 -device spapr-vty,chardev=charserial0,reg=0x20000000 \ +-chardev pty,id=charserial1 -device spapr-vty,chardev=charserial1,reg=0x30001000 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args index 5fe0c88..c5bef9f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args @@ -1 +1,9 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-ppc64 -S -M pseries -m 512 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device spapr-vscsi,id=scsi0,reg=0x2000 -device spapr-vscsi,id=scsi1,reg=0x3000 -drive file=/tmp/scsidisk.img,if=none,id=drive-scsi1-0-0 -device scsi-disk,bus=scsi1.0,scsi-id=0,drive=drive-scsi1-0-0,id=scsi1-0-0 -chardev pty,id=charserial0 -device spapr-vty,chardev=charserial0,reg=0x30000000 -chardev pty,id=charserial1 -device spapr-vty,chardev=charserial1,reg=0x30001000 -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-ppc64 \ +-S -M pseries -m 512 -smp 1 -nographic -nodefconfig -nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ +-device spapr-vscsi,id=scsi0,reg=0x2000 -device spapr-vscsi,id=scsi1,reg=0x3000 \ +-drive file=/tmp/scsidisk.img,if=none,id=drive-scsi1-0-0-0 \ +-device scsi-disk,bus=scsi1.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi1-0-0-0,id=scsi1-0-0-0 \ +-chardev pty,id=charserial0 -device spapr-vty,chardev=charserial0,reg=0x30000000 \ +-chardev pty,id=charserial1 -device spapr-vty,chardev=charserial1,reg=0x30001000 -usb diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml index 17b59d8..9456335 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml @@ -26,7 +26,7 @@ </disk> <controller type='usb' index='0'/> <controller type='ide' index='0'/> - <controller type='scsi' index='0'/> + <controller type='scsi' index='0' model='lsilogic'/> <memballoon model='virtio'/> </devices> </domain> -- 1.7.7.3
--- tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml | 2 +- tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml | 2 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml | 2 +- tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml | 2 +- tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml | 2 +- tests/vmx2xmldata/vmx2xml-scsi-driver.xml | 6 +++--- tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml index 7eb3676..00fbf4e 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml @@ -14,7 +14,7 @@ <disk type='block' device='cdrom'> <source dev='/dev/scd0'/> <target dev='sda' bus='scsi'/> - <address type='drive' controller='0' bus='0' unit='0'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0'/> <video> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml index df1e7c4..e8f2e9f 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml @@ -14,7 +14,7 @@ <disk type='file' device='cdrom'> <source file='[datastore] directory/cdrom.iso'/> <target dev='sda' bus='scsi'/> - <address type='drive' controller='0' bus='0' unit='0'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0'/> <video> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml index 23fc1f6..6dcc2d7 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml @@ -21,7 +21,7 @@ <disk type='file' device='cdrom'> <source file='[datastore] directory/Debian1-cdrom.iso'/> <target dev='sdp' bus='scsi'/> - <address type='drive' controller='1' bus='0' unit='0'/> + <address type='drive' controller='1' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <source file='/vmimages/tools-isoimages/linux.iso'/> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml index 0c4e4d5..db93eb0 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml @@ -15,7 +15,7 @@ <disk type='file' device='disk'> <source file='[datastore] directory/el6-test-000001.vmdk'/> <target dev='sda' bus='scsi'/> - <address type='drive' controller='0' bus='0' unit='0'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <source file='/usr/lib/vmware/isoimages/linux.iso'/> diff --git a/tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml b/tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml index 189e72d..4e90388 100644 --- a/tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml +++ b/tests/vmx2xmldata/vmx2xml-harddisk-scsi-file.xml @@ -14,7 +14,7 @@ <disk type='file' device='disk'> <source file='[datastore] directory/harddisk.vmdk'/> <target dev='sda' bus='scsi'/> - <address type='drive' controller='0' bus='0' unit='0'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0'/> <video> diff --git a/tests/vmx2xmldata/vmx2xml-scsi-driver.xml b/tests/vmx2xmldata/vmx2xml-scsi-driver.xml index 8fa907b..34693a4 100644 --- a/tests/vmx2xmldata/vmx2xml-scsi-driver.xml +++ b/tests/vmx2xmldata/vmx2xml-scsi-driver.xml @@ -14,7 +14,7 @@ <disk type='file' device='disk'> <source file='[datastore] directory/harddisk1.vmdk'/> <target dev='sda' bus='scsi'/> - <address type='drive' controller='0' bus='0' unit='0'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='disk'> <source file='[datastore] directory/harddisk2.vmdk'/> @@ -24,12 +24,12 @@ <disk type='file' device='disk'> <source file='[datastore] directory/harddisk3.vmdk'/> <target dev='sdae' bus='scsi'/> - <address type='drive' controller='2' bus='0' unit='0'/> + <address type='drive' controller='2' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='disk'> <source file='[datastore] directory/harddisk4.vmdk'/> <target dev='sdat' bus='scsi'/> - <address type='drive' controller='3' bus='0' unit='0'/> + <address type='drive' controller='3' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0' model='buslogic'/> <controller type='scsi' index='1' model='lsilogic'/> diff --git a/tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml b/tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml index e5b8934..d2b1cb7 100644 --- a/tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml +++ b/tests/vmx2xmldata/vmx2xml-scsi-writethrough.xml @@ -15,7 +15,7 @@ <driver cache='writethrough'/> <source file='[datastore] directory/harddisk.vmdk'/> <target dev='sda' bus='scsi'/> - <address type='drive' controller='0' bus='0' unit='0'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0' model='buslogic'/> <video> -- 1.7.7.3
--- .../qemuxml2argv-disk-scsi-virtio-scsi.args | 9 +++++ .../qemuxml2argv-disk-scsi-virtio-scsi.xml | 32 ++++++++++++++++++++ .../qemuxml2argv-disk-scsi-vscsi.args | 8 +++++ .../qemuxml2argv-disk-scsi-vscsi.xml | 32 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++ tests/qemuxml2xmltest.c | 4 ++- 6 files changed, 88 insertions(+), 1 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-virtio-scsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-virtio-scsi.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-vscsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-vscsi.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-virtio-scsi.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-virtio-scsi.args new file mode 100644 index 0000000..de8d5a6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-virtio-scsi.args @@ -0,0 +1,9 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \ +virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 -drive file=/dev/HostVG/QEMUGuest1,\ +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=/tmp/scsidisk.img,if=none,\ +id=drive-scsi0-0-0-0 -device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 -usb -device virtio-balloon-pci,\ +id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-virtio-scsi.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-virtio-scsi.xml new file mode 100644 index 0000000..ce8b1a6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-virtio-scsi.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>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'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <disk type='file' device='disk'> + <source file='/tmp/scsidisk.img'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-vscsi.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-vscsi.args new file mode 100644 index 0000000..8f2dd35 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-vscsi.args @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device spapr-vscsi,id=scsi0,\ +bus=pci.0,addr=0x3 -drive file=/dev/HostVG/QEMUGuest1,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=/tmp/scsidisk.img,if=none,id=drive-scsi0-0-0-0 \ +-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ +-usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-vscsi.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-vscsi.xml new file mode 100644 index 0000000..7368d3b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-vscsi.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>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'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <disk type='file' device='disk'> + <source file='/tmp/scsidisk.img'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <controller type='scsi' index='0' model='ibmvscsi'/> + <controller type='usb' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 13d364e..fcffc27 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -457,6 +457,10 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-scsi-device-auto", false, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("disk-scsi-vscsi", false, + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("disk-scsi-virtio-scsi", false, + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-sata-device", false, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_ICH9_AHCI); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c1b2b14..3907c90 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -40,7 +40,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool live) if (!(actual = virDomainDefFormat(def, VIR_DOMAIN_XML_SECURE))) goto fail; - + //printf("actual = %s\n", actual); if (STRNEQ(outXmlData, actual)) { virtTestDifference(stderr, outXmlData, actual); goto fail; @@ -150,6 +150,8 @@ mymain(void) DO_TEST("disk-drive-cache-v1-wb"); DO_TEST("disk-drive-cache-v1-none"); DO_TEST("disk-scsi-device"); + DO_TEST("disk-scsi-vscsi"); + DO_TEST("disk-scsi-virtio-scsi"); DO_TEST("graphics-listen-network"); DO_TEST("graphics-vnc"); DO_TEST("graphics-vnc-sasl"); -- 1.7.7.3
participants (2)
-
Osier Yang -
Paolo Bonzini