在 2018/8/17 上午12:31, Andrea Bolognani 写道:
On Tue, 2018-08-07 at 17:10 +0800, Yi Min Zhao wrote:
[...]
+char *
+qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ virBufferAddLit(&buf, "zpci");
+ virBufferAsprintf(&buf, ",uid=%u", dev->addr.pci.zpci->zpci_uid);
+ virBufferAsprintf(&buf, ",fid=%u", dev->addr.pci.zpci->zpci_fid);
+ virBufferAsprintf(&buf, ",target=%s", dev->alias);
+ virBufferAsprintf(&buf, ",id=zpci%u", dev->addr.pci.zpci->zpci_uid);
Just making sure: is the uid a good choice for naming the zpci
device? I would perhaps expect the fid to be in there as well,
but since both have to be unique (right?) I guess it doesn't
really matter...
Either uid or fid, the value must be unique. But uid is defined by user.
Of course, we also give the permission to define fid. But uid is more
appropriate.
+
+ if (virBufferCheckError(&buf) < 0) {
+ virBufferFreeAndReset(&buf);
+ return NULL;
+ }
+
+ return virBufferContentAndReset(&buf);
+}
+
+int
+qemuCheckDeviceIsZPCI(virDomainDeviceInfoPtr info)
Based on the name, this is a predicate: it should return a
boolean value rather than an int.
0 means it's not zpci. 1 means it's zpci. -1 means error.
+{
+ if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
+ info->pciAddressExtFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) {
+ return 1;
+ }
+
+ if (info->addr.pci.zpci) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This QEMU doesn't support zpci devices"));
+ return -1;
+ }
Not sure about this second check. I guess the logic is supposed to
be that, when passed a "proper" zPCI device, the function would
have returned with the first check, and if we find a zPCI address
after that it's an error. Makes sense, but it's kinda obfuscated
and I'm not sure the error message is accurate: can it really only
be a problem of the QEMU binary not supporting the zPCI feature,
or could we have gotten here because the user is trying to assing
zPCI addresses to devices that don't support them?
Yes, it's because user could define zpci address in xml but
there's no zpci support.
Either way, calling a predicate should never result in an error
being reported, so you need to move this check somewhere else.
Acutally I have found out a proper place to add this check for a long time.
So this is the best place to add, at least I think for now.
[...]
+++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.xml
@@ -0,0 +1,18 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219100</memory>
+ <os>
+ <type arch='s390x' machine='s390-ccw-virtio'>hvm</type>
+ </os>
+ <devices>
+ <emulator>/usr/bin/qemu-system-s390x</emulator>
+ <hostdev mode='subsystem' type='pci'>
+ <driver name='vfio'/>
+ <source>
+ <address domain='0x0000' bus='0x00' slot='0x00' function='0x0'/>
+ </source>
+ <address type='pci'/>
+ </hostdev>
+ </devices>
+</domain>
This test case would have been a great addition to the previous
commit, where you actually introduced the ability to automatically
allocate zPCI addresses...
Another thing that I forgot to ask earlier and this is as good a
time as any: once zPCI support has been merged, will users have
to opt-in to it using
<address type='pci'/>
or will they get zPCI devices by default? And if so, will it still
be possible for them to get CCW devices instead if wanted?
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
