[libvirt] [PATCHv4 3/5] S390: QEMU driver support for CCW addresses

[Daniel P. Berrange]

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 at linux.vnet.ibm.com>
> ---
> 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 :|