On Mon, Jul 20, 2020 at 12:22:33PM +0200, Thomas Huth wrote: > qemuAgentFSInfoToPublic() currently only sets the devAlias for PCI devices. > However, the QEMU guest agent could also provide the device name in the > "dev" field of the response for other devices instead (well, at least after > fixing another problem in the current QEMU guest agent...). So if creating > the devAlias from the PCI information failed, let's fall back to the name > provided by the guest agent. This helps to fix the empty "Target" fields > that occur when running "virsh domfsinfo" on s390x where CCW devices are > used for the guest instead of PCI devices. > > Also add a proper debug message here in case we completely failed to set the > device alias, since this problem here was very hard to debug: The only two > error messages that I've seen were "Unable to get filesystem information" > and "Unable to encode message payload" - which only indicates that something > went wrong in the RPC call. No debug message indicated the real problem, so > I had to learn the hard way why the RPC call failed (it apparently does not > like devAlias left to be NULL) and where the real problem comes from. > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1755075 > Signed-off-by: Thomas Huth <thuth@xxxxxxxxxx> > --- > src/qemu/qemu_driver.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index d185666ed8..e45c61ee20 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -21935,14 +21935,17 @@ qemuAgentFSInfoToPublic(qemuAgentFSInfoPtr agent, > qemuAgentDiskInfoPtr agentdisk = agent->disks[i]; > virDomainDiskDefPtr diskDef; > > - if (!(diskDef = virDomainDiskByAddress(vmdef, > - &agentdisk->pci_controller, > - agentdisk->bus, > - agentdisk->target, > - agentdisk->unit))) > - continue; > - > - ret->devAlias[i] = g_strdup(diskDef->dst); > + diskDef = virDomainDiskByAddress(vmdef, > + &agentdisk->pci_controller, > + agentdisk->bus, > + agentdisk->target, > + agentdisk->unit); Hmmm, even on x86 I think the original code is broken, or at least fragile. The libvirt "bus" number is not guaranteed to match the guest kernel PCI "bus" number. I wonder if this has been validated for complex PCI-Express topologies. > + if (diskDef != NULL) > + ret->devAlias[i] = g_strdup(diskDef->dst); > + else if (agentdisk->devnode != NULL) > + ret->devAlias[i] = g_strdup(agentdisk->devnode); In the linked BZ the "devnode" field is not provided either. You mention a related QEMU patch, which I guess fixes that issue in QEMU ? Can you point to that fix. > + else > + VIR_DEBUG("Missing devnode name for '%s'.", ret->mountpoint); > } Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
