Re: [libvirt] [PATCH] qemu: monitor: Add memory balloon support for virtio-ccw

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(a)linux.vnet.ibm.com&gt; >> Reviewed-by: Christian Borntraeger <borntraeger(a)de.ibm.com&gt; >> --- >> 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"?

> > 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 >