On Tue, Mar 05, 2013 at 04:44:21PM +0100, Viktor Mihajlovski wrote:
> This commit adds the QEMU driver support for CCW addresses. The
> current QEMU only allows virtio devices to be attached to the
> CCW bus. We named the new capability indicating that support
> QEMU_CAPS_VIRTIO_CCW accordingly.
>
> The fact that CCW devices can only be assigned to domains with a
> machine type of s390-ccw-virtio requires a few extra checks for
> machine type in qemu_command.c on top of querying
> QEMU_CAPS_VIRTIO_{CCW|S390}.
>
> The majority of the new functions deals with CCW address generation
> and management.
>
> Signed-off-by: Viktor Mihajlovski <mihajlov@xxxxxxxxxxxxxxxxxx>
> ---
> V2 Changes
> - rebase, mainly addressing the rename of qemuCapsXXX to virQEMUCapsXXX
>
> V3 Changes
> - revert machine based capability detection
> - check the machine type in conjunction with s390-specific capability
> tests in qemu_command.c
> - remove useless paranoia check in qemu_command.c
>
> V4 Changes
> - replace CCW address field name 'schid' with 'devno'
>
> src/qemu/qemu_capabilities.c | 7 +-
> src/qemu/qemu_capabilities.h | 1 +
> src/qemu/qemu_command.c | 279 +++++++++++++++++++++++++++++++++++++++---
> src/qemu/qemu_command.h | 6 +
> src/qemu/qemu_domain.c | 1 +
> src/qemu/qemu_domain.h | 3 +
> src/qemu/qemu_process.c | 3 +
> 7 files changed, 280 insertions(+), 20 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 40022c1..79cfdb3 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -210,6 +210,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>
> "rng-random", /* 130 */
> "rng-egd",
> + "virtio-ccw"
> );
>
> struct _virQEMUCaps {
> @@ -1318,6 +1319,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
> { "usb-hub", QEMU_CAPS_USB_HUB },
> { "ich9-ahci", QEMU_CAPS_ICH9_AHCI },
> { "virtio-blk-s390", QEMU_CAPS_VIRTIO_S390 },
> + { "virtio-blk-ccw", QEMU_CAPS_VIRTIO_CCW },
> { "sclpconsole", QEMU_CAPS_SCLP_S390 },
> { "lsi53c895a", QEMU_CAPS_SCSI_LSI },
> { "virtio-scsi-pci", QEMU_CAPS_VIRTIO_SCSI_PCI },
> @@ -1338,7 +1340,6 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
> { "rng-egd", QEMU_CAPS_OBJECT_RNG_EGD },
> };
>
> -
> static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
> { "multifunction", QEMU_CAPS_PCI_MULTIFUNCTION },
> { "bootindex", QEMU_CAPS_BOOTINDEX },
> @@ -1393,6 +1394,10 @@ static struct virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = {
> ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBlk) },
> { "virtio-net-pci", virQEMUCapsObjectPropsVirtioNet,
> ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioNet) },
> + { "virtio-blk-ccw", virQEMUCapsObjectPropsVirtioBlk,
> + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBlk) },
> + { "virtio-net-ccw", virQEMUCapsObjectPropsVirtioNet,
> + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioNet) },
> { "virtio-blk-s390", virQEMUCapsObjectPropsVirtioBlk,
> ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioBlk) },
> { "virtio-net-s390", virQEMUCapsObjectPropsVirtioNet,
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index a895867..5c5dc5a 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -171,6 +171,7 @@ enum virQEMUCapsFlags {
> QEMU_CAPS_OBJECT_RNG_RANDOM = 130, /* the rng-random backend for
> virtio rng */
> QEMU_CAPS_OBJECT_RNG_EGD = 131, /* EGD protocol daemon for rng */
> + QEMU_CAPS_VIRTIO_CCW = 132, /* -device virtio-*-ccw */
>
> 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 201fac1..cc64c17 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -799,6 +799,100 @@ no_memory:
> return -1;
> }
>
> +/* S390 ccw bus support */
> +
> +struct _qemuDomainCCWAddressSet {
> + virHashTablePtr defined;
Too much whitespace ^^^^^^^^^^^^^^^^
> + virDomainDeviceCCWAddress next;
> +};
> +
> +static char*
> +qemuCCWAddressAsString(virDomainDeviceCCWAddressPtr addr)
> +{
> + char *addrstr = NULL;
> +
> + if (virAsprintf(&addrstr, "%x.%x.%04x",
Should we zero-pad the first two fields too, or is it common
to only pad the last field in ccw addresses ?
> + addr->cssid,
> + addr->ssid,
> + addr->devno) < 0) {
> + virReportOOMError();
> + return NULL;
> + }
> +
> + return addrstr;
> +}
> +
> -static void
> -qemuDomainAssignS390Addresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
> +static int
> +qemuDomainCCWAddressAllocate(virDomainDefPtr def ATTRIBUTE_UNUSED,
> + virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
> + virDomainDeviceInfoPtr info,
> + void * data)
> +{
> + return qemuDomainCCWAddressAssign(info,
> + (qemuDomainCCWAddressSetPtr)data,
You don't need to cast 'void *' in C.
> + true);
> +}
> +
> +static int
> +qemuDomainCCWAddressValidate(virDomainDefPtr def ATTRIBUTE_UNUSED,
> + virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
> + virDomainDeviceInfoPtr info,
> + void * data)
> +{
> + return qemuDomainCCWAddressAssign(info,(qemuDomainCCWAddressSetPtr)data,
Likewise
ACK, since there's no bugs in my comments, just style issues.
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list