[PATCH] qemu_driver.c: Fix domfsinfo for non-PCI device information from guest agent
Daniel P. Berrangé
berrange at redhat.com
Wed Sep 2 16:34:20 UTC 2020
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 at redhat.com>
> >> ---
> >> 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 at redhat.com>
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 :|
More information about the libvir-list
mailing list