On Wed, Sep 02, 2020 at 06:26:02PM +0200, Thomas Huth wrote: > On 07/08/2020 18.22, Thomas Huth wrote: > > On 20/07/2020 12.22, 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); > >> + if (diskDef != NULL) > >> + ret->devAlias[i] = g_strdup(diskDef->dst); > >> + else if (agentdisk->devnode != NULL) > >> + ret->devAlias[i] = g_strdup(agentdisk->devnode); > >> + else > >> + VIR_DEBUG("Missing devnode name for '%s'.", ret->mountpoint); > >> } > >> > >> return ret; > >> > > > > Friendly ping! > > > > Any more comments here? Is it ok this way or shall I change the order? > > Ping again! Is the patch ok, or do I have to rework it? Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> and i'll push it shortly. The related QEMU fix is also queued for 5.2 I see. 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 :|
