1:54 a.m.
On qemu-kvm-0.15.1, it supports only lsi scsi controller model.
On qemu-kvm-0.12.1.2, it supports only virtio-scsi-pci scsi model
On qemu 1.1.50, it supports both.
So, instead of using the lsilogic model by default, the patch tries
to check which model the current QEMU supports, then choose it, lsi
has the priority.
If a scsi model is given in XML explicitly, we try to check if
the underlying QEMU supports it or not, raise an error on checking
failure.
Guannan Ren(2)
(1/2)qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers
(2/2)test: add lsi and virtio-scsi qemu caps in testcases.
src/qemu/qemu_capabilities.c | 7 ---
src/qemu/qemu_capabilities.h | 2 -
src/qemu/qemu_command.c | 88 +++++++++++------------------------------
src/qemu/qemu_command.h | 3 +-
tests/qemuhelptest.c | 10 +---
tests/qemuxml2argvtest.c | 16 +++-----
6 files changed, 34 insertions(+), 92 deletions(-)
1:54 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: add two qemu caps for lsi and virtio-scsi SCSI controllers
Rename qemuDefaultScsiControllerModel to qemuCheckScsiControllerModel.
When scsi model is given explicitly in XML(model > 0) checking if the
underlying QEMU support it or not first, raise error on checking failure.
If the model is not given(mode <= 0), return lsi or virtio-scsi
which is supported, lsi take the priority.
---
src/qemu/qemu_capabilities.c | 7 +++
src/qemu/qemu_capabilities.h | 2 +
src/qemu/qemu_command.c | 88 ++++++++++++++++++++++++++++++-----------
src/qemu/qemu_command.h | 3 +-
4 files changed, 75 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 85c49a2..8282ad8 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -169,6 +169,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
"virtio-s390",
"balloon-event",
+ "lsi", /*100*/
+ "virtio-scsi-pci",
);
struct qemu_feature_flags {
@@ -1450,6 +1452,11 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr flags)
strstr(str, "name \"virtio-serial-s390\""))
qemuCapsSet(flags, QEMU_CAPS_VIRTIO_S390);
+ if (strstr(str, "name \"lsi53c895a\""))
+ qemuCapsSet(flags, QEMU_CAPS_SCSI_LSI);
+ if (strstr(str, "name \"virtio-scsi-pci\""))
+ qemuCapsSet(flags, QEMU_CAPS_VIRIO_SCSI_PCI);
+
/* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */
if (!qemuCapsGet(flags, QEMU_CAPS_CHARDEV_SPICEVMC) &&
strstr(str, "name \"spicevmc\""))
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index e8251dc..d2d68a9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -135,6 +135,8 @@ enum qemuCapsFlags {
QEMU_CAPS_NEC_USB_XHCI = 97, /* -device nec-usb-xhci */
QEMU_CAPS_VIRTIO_S390 = 98, /* -device virtio-*-s390 */
QEMU_CAPS_BALLOON_EVENT = 99, /* Async event for balloon changes */
+ QEMU_CAPS_SCSI_LSI = 100, /* -device lsi */
+ QEMU_CAPS_VIRIO_SCSI_PCI = 101, /* -device virtio-scsi-pci */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6ad65a6..6a4578d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -469,19 +469,60 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk)
}
static int
-qemuDefaultScsiControllerModel(virDomainDefPtr def) {
- if (STREQ(def->os.arch, "ppc64") &&
- STREQ(def->os.machine, "pseries")) {
- return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+qemuCheckScsiControllerModel(virDomainDefPtr def,
+ virBitmapPtr qemuCaps,
+ int *model)
+{
+ if (*model > 0) {
+ switch (*model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC:
+ if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This QEMU doesn't support "
+ "lsi scsi controller"));
+ return -1;
+ }
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
+ if (!qemuCapsGet(qemuCaps, QEMU_CAPS_VIRIO_SCSI_PCI)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This QEMU doesn't support "
+ "virtio scsi controller"));
+ return -1;
+ }
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
+ /*TODO: need checking work here if necessary */
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported controller model: %s"),
+ virDomainControllerModelSCSITypeToString(*model));
+ return -1;
+ }
} else {
- return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+ if (qemuCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) {
+ *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+ } else if (qemuCapsGet(qemuCaps, QEMU_CAPS_VIRIO_SCSI_PCI)) {
+ *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
+ } else if (STREQ(def->os.arch, "ppc64") &&
+ STREQ(def->os.machine, "pseries")) {
+ *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to determine model for scsi
controller"));
+ return -1;
+ }
}
+
+ return 0;
}
/* Our custom -drive naming scheme used with id= */
static int
qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def,
- virDomainDiskDefPtr disk)
+ virDomainDiskDefPtr disk,
+ virBitmapPtr qemuCaps)
{
const char *prefix = virDomainDiskBusTypeToString(disk->bus);
int controllerModel = -1;
@@ -491,11 +532,10 @@ qemuAssignDeviceDiskAliasCustom(virDomainDefPtr def,
controllerModel =
virDomainDiskFindControllerModel(def, disk,
VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
- }
- if (controllerModel == -1 ||
- controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO)
- controllerModel = qemuDefaultScsiControllerModel(def);
+ if ((qemuCheckScsiControllerModel(def, qemuCaps, &controllerModel)) <
0)
+ return -1;
+ }
if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI ||
controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) {
@@ -533,7 +573,7 @@ qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef,
{
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE))
- return qemuAssignDeviceDiskAliasCustom(vmdef, def);
+ return qemuAssignDeviceDiskAliasCustom(vmdef, def, qemuCaps);
else
return qemuAssignDeviceDiskAliasFixed(def);
} else {
@@ -850,9 +890,10 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def, virDomainDeviceInfoPtr
info,
return 0;
}
-int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def)
+int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
+ virBitmapPtr qemuCaps)
{
- int i, rc;
+ int i, rc = -1;
int model;
/* Default values match QEMU. See spapr_(llan|vscsi|vty).c */
@@ -869,9 +910,10 @@ int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def)
for (i = 0 ; i < def->ncontrollers; i++) {
model = def->controllers[i]->model;
- if (model == -1 &&
- def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
- model = qemuDefaultScsiControllerModel(def);
+ if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
+ if ((qemuCheckScsiControllerModel(def, qemuCaps, &model)) < 0)
+ return rc;
+
if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI &&
def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
def->controllers[i]->info.type =
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
@@ -1059,7 +1101,7 @@ int qemuDomainAssignAddresses(virDomainDefPtr def,
{
int rc;
- rc = qemuDomainAssignSpaprVIOAddresses(def);
+ rc = qemuDomainAssignSpaprVIOAddresses(def, qemuCaps);
if (rc)
return rc;
@@ -2434,9 +2476,8 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
controllerModel =
virDomainDiskFindControllerModel(def, disk,
VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
- if (controllerModel == -1 ||
- controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO)
- controllerModel = qemuDefaultScsiControllerModel(def);
+ if ((qemuCheckScsiControllerModel(def, qemuCaps, &controllerModel)) < 0)
+ goto error;
if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) {
if (disk->info.addr.drive.target != 0) {
@@ -2765,10 +2806,9 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
switch (def->type) {
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
model = def->model;
- if (model == -1 ||
- model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) {
- model = qemuDefaultScsiControllerModel(domainDef);
- }
+ if ((qemuCheckScsiControllerModel(domainDef, qemuCaps, &model)) < 0)
+ return NULL;
+
switch (model) {
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
virBufferAddLit(&buf, "virtio-scsi-pci");
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 3ccf4d7..7068a81 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -178,7 +178,8 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps,
int qemuDomainAssignAddresses(virDomainDefPtr def,
virBitmapPtr qemuCaps,
virDomainObjPtr);
-int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def);
+int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def,
+ virBitmapPtr qemuCaps);
int qemuDomainAssignPCIAddresses(virDomainDefPtr def,
virBitmapPtr qemuCaps,
--
1.7.7.6
1:54 a.m.
New subject: [libvirt] [PATCH 2/2] test: add lsi and virtio-scsi qemu caps in testcases.
---
tests/qemuhelptest.c | 10 +++++++---
tests/qemuxml2argvtest.c | 16 ++++++++++------
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 012ba26..62afabf 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -482,7 +482,8 @@ mymain(void)
QEMU_CAPS_PCI_ROMBAR,
QEMU_CAPS_NO_ACPI,
QEMU_CAPS_VIRTIO_BLK_SG_IO,
- QEMU_CAPS_CPU_HOST);
+ QEMU_CAPS_CPU_HOST,
+ QEMU_CAPS_SCSI_LSI);
DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -677,7 +678,8 @@ mymain(void)
QEMU_CAPS_FSDEV_WRITEOUT,
QEMU_CAPS_SCSI_BLOCK,
QEMU_CAPS_SCSI_CD,
- QEMU_CAPS_IDE_CD);
+ QEMU_CAPS_IDE_CD,
+ QEMU_CAPS_SCSI_LSI);
DO_TEST("qemu-1.1.0", 1001000, 0, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -754,7 +756,9 @@ mymain(void)
QEMU_CAPS_IDE_CD,
QEMU_CAPS_NO_USER_CONFIG,
QEMU_CAPS_HDA_MICRO,
- QEMU_CAPS_NEC_USB_XHCI);
+ QEMU_CAPS_NEC_USB_XHCI,
+ QEMU_CAPS_SCSI_LSI,
+ QEMU_CAPS_VIRIO_SCSI_PCI);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 39fcd9f..af996f3 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -469,16 +469,19 @@ mymain(void)
DO_TEST("disk-usb-device",
QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
DO_TEST("disk-scsi-device",
- QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
+ QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_SCSI_LSI);
DO_TEST("disk-scsi-device-auto",
- QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
+ QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_SCSI_LSI);
DO_TEST("disk-scsi-disk-split",
QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
- QEMU_CAPS_SCSI_CD);
+ QEMU_CAPS_SCSI_CD, QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRIO_SCSI_PCI);
DO_TEST("disk-scsi-vscsi",
QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
DO_TEST("disk-scsi-virtio-scsi",
- QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
+ QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_VIRIO_SCSI_PCI);
DO_TEST("disk-sata-device",
QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE,
QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_ICH9_AHCI);
@@ -508,7 +511,8 @@ mymain(void)
DO_TEST("disk-scsi-lun-passthrough",
QEMU_CAPS_DRIVE,
QEMU_CAPS_DEVICE,
- QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_VIRTIO_BLK_SG_IO);
+ QEMU_CAPS_SCSI_BLOCK, QEMU_CAPS_VIRTIO_BLK_SG_IO,
+ QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRIO_SCSI_PCI);
DO_TEST("graphics-vnc", NONE);
DO_TEST("graphics-vnc-socket", NONE);
@@ -768,7 +772,7 @@ mymain(void)
DO_TEST("multifunction-pci-device",
QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
- QEMU_CAPS_PCI_MULTIFUNCTION);
+ QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_SCSI_LSI);
DO_TEST("monitor-json", QEMU_CAPS_DEVICE,
QEMU_CAPS_CHARDEV, QEMU_CAPS_MONITOR_JSON, QEMU_CAPS_NODEFCONFIG);
--
1.7.7.6
6:37 a.m.
On Tue, Aug 07, 2012 at 02:54:33PM +0800, Guannan Ren wrote:
On qemu-kvm-0.15.1, it supports only lsi scsi controller model.
On qemu-kvm-0.12.1.2, it supports only virtio-scsi-pci scsi model
This doesn't make sense. virtio-scsi did not exist until long
after qemu-kvm-0.12.1.2.
On qemu 1.1.50, it supports both.
So, instead of using the lsilogic model by default, the patch tries
to check which model the current QEMU supports, then choose it, lsi
has the priority.
I'm not convinced we should ever be trying to use virtio scsi by
default, given its wide lack of guest support. If LSI scsi does
not exist, just report an error IMHO, and let virtio scsi require
an explicit config choice.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:53 a.m.
On 08/07/2012 07:37 PM, Daniel P. Berrange wrote:
On Tue, Aug 07, 2012 at 02:54:33PM +0800, Guannan Ren wrote:
> On qemu-kvm-0.15.1, it supports only lsi scsi controller model.
> On qemu-kvm-0.12.1.2, it supports only virtio-scsi-pci scsi model
This doesn't make sense. virtio-scsi did not exist until long
after qemu-kvm-0.12.1.2.
This is the version on my RHEL6.3 , maybe not upstream version code.
# rpm -qa|grep kvm
qemu-kvm-0.12.1.2-2.302.el6.x86_64
# /usr/libexec/qemu-kvm --version
QEMU PC emulator version 0.12.1 (qemu-kvm-0.12.1.2)
> On qemu 1.1.50, it supports both.
> So, instead of using the lsilogic model by default, the patch tries
> to check which model the current QEMU supports, then choose it, lsi
> has the priority.
I'm not convinced we should ever be trying to use virtio scsi by
default, given its wide lack of guest support. If LSI scsi does
not exist, just report an error IMHO, and let virtio scsi require
an explicit config choice.
Get it. the virtio-scsi is not used widely.
Thanks for the review.
Guannan
4524
days inactive
4524
days old
4 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Guannan Ren