On 03/13/2013 09:40 AM, Daniel P. Berrange wrote:
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}.
+struct _qemuDomainCCWAddressSet { + virHashTablePtr defined;
Too much whitespace ^^^^^^^^^^^^^^^^
I cleaned this up,
+ 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 ?
but left this alone. You can submit a followup patch if it is necessary.
-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.
I cleaned this up as well.
ACK, since there's no bugs in my comments, just style issues.
I'll push this, along with the other ACK'd patches in the series, shortly. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org