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

[Ján Tomko]

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 at linux.vnet.ibm.com>
> >> Reviewed-by: Christian Borntraeger <borntraeger at de.ibm.com>
> >> ---
> >>   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
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150605/5b638f86/attachment-0001.sig>