On Wed, Jun 03, 2015 at 10:51:04AM +0200, Boris Fiuczynski wrote: > On 06/02/2015 05:56 PM, Ján Tomko wrote: > > On Tue, Jun 02, 2015 at 11:27:35AM +0200, Boris Fiuczynski wrote: > >> The search for the memory ballon driver object is extended by a > >> second known name "virtio-ballon-ccw" in support for virtio-ccw. > >> > >> Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx> > >> Reviewed-by: Christian Borntraeger <borntraeger@xxxxxxxxxx> > >> --- > >> src/qemu/qemu_monitor.c | 9 +++++---- > >> 1 file changed, 5 insertions(+), 4 deletions(-) > >> > >> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c > >> index f959b74..1a88329 100644 > >> --- a/src/qemu/qemu_monitor.c > >> +++ b/src/qemu/qemu_monitor.c > >> @@ -1141,9 +1141,9 @@ qemuMonitorFindObjectPath(qemuMonitorPtr mon, > >> > >> > >> /** > >> - * Search the qom objects for the balloon driver object by it's known name > >> - * of "virtio-balloon-pci". The entry for the driver will be found by using > >> - * function "qemuMonitorFindObjectPath". > >> + * Search the qom objects for the balloon driver object by it's known names > > > > s/it's/its/ > Will fix > > > > >> + * of "virtio-balloon-pci" or "virtio-ballon-ccw". The entry for the driver > >> + * will be found by using function "qemuMonitorFindObjectPath". > >> * > >> * Once found, check the entry to ensure it has the correct property listed. > >> * If it does not, then obtaining statistics from QEMU will not be possible. > >> @@ -1183,7 +1183,8 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon, > >> return -1; > >> } > >> > >> - if (qemuMonitorFindObjectPath(mon, curpath, "virtio-balloon-pci", &path) < 0) > >> + if (qemuMonitorFindObjectPath(mon, curpath, "virtio-balloon-pci", &path) < 0 && > >> + qemuMonitorFindObjectPath(mon, curpath, "virtio-balloon-ccw", &path) < 0) > >> return -1; > > > > qemuMonitorFindObjectPath can return: > > 0 - Found > > -1 - Error bail out > > -2 - Not found > > > > But it only reports an error when returning -1. There is a > > (pre-existing) missing virReportError in that case. > When looking at the code how qemuMonitorFindBalloonObjectPath was used I > got the impression that this was intentional since there is > mon->balloonpath and mon->ballooninit that are checked after the first > unsuccessful call of qemuMonitorFindBalloonObjectPath that report the > error "Cannot determine balloon device path". That is why I stuck with > that idea of just reporting "Cannot determine balloon device path". > > Do you suggest to report additional errors when the > qemuMonitorFindObjectPath fails with "Error bail out", "Not found (PCI)" > and "Not found (CCW)" and later always "Cannot determine balloon device > path"? > Right, the error should not be needed and it seems to be omitted on purpose. And the return value of qemuMonitorFindBalloonObjectPath is ignored. I've sent a series that makes it more obvious: https://www.redhat.com/archives/libvir-list/2015-June/msg00207.html Still, it only makes sense to look for ccw balloon if the pci one was not found. Jan > > > > Looking for the ccw balloon only makes sense when the pci one was not > > found. A fatal error (-1) when finding the PCI balloon was not found > > will very probably be fatal for CCW as well. > > > > Jan > >
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list