Instead of trying newer APIs first and only falling back to
deprecated ones if those are not available, go straight for
the older APIs.

Running 'virsh list' will now cause warnings to be displayed.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 tools/virsh-domain-monitor.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 3a2636377d..f2f2944a8a 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1609,6 +1609,8 @@ virshDomainListCollect(vshControl *ctl, unsigned int flags)
     int mansave;
     virshControlPtr priv = ctl->privData;
 
+    goto fallback;
+
     /* try the list with flags support (0.9.13 and later) */
     if ((ret = virConnectListAllDomains(priv->conn, &list->domains,
                                         flags)) >= 0) {
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Oct 02, 2018 at 16:14:46 +0200, Andrea Bolognani wrote:
> Instead of trying newer APIs first and only falling back to
> deprecated ones if those are not available, go straight for
> the older APIs.
> 
> Running 'virsh list' will now cause warnings to be displayed.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  tools/virsh-domain-monitor.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 3a2636377d..f2f2944a8a 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -1609,6 +1609,8 @@ virshDomainListCollect(vshControl *ctl, unsigned int flags)
>      int mansave;
>      virshControlPtr priv = ctl->privData;
>  
> +    goto fallback;
> +

Obvious NACK since you did not make the patch look as a reproducer case
to prove the code changes work.

I suggest adding a all-caps warning to the subject and NOT adding a
Sign-off for reproducers not meant to be pushed.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, 2018-10-02 at 16:30 +0200, Peter Krempa wrote:
> On Tue, Oct 02, 2018 at 16:14:46 +0200, Andrea Bolognani wrote:
> > Instead of trying newer APIs first and only falling back to
> > deprecated ones if those are not available, go straight for
> > the older APIs.
> > 
> > Running 'virsh list' will now cause warnings to be displayed.
> > 
> > Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> > ---
> >  tools/virsh-domain-monitor.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> > index 3a2636377d..f2f2944a8a 100644
> > --- a/tools/virsh-domain-monitor.c
> > +++ b/tools/virsh-domain-monitor.c
> > @@ -1609,6 +1609,8 @@ virshDomainListCollect(vshControl *ctl, unsigned int flags)
> >      int mansave;
> >      virshControlPtr priv = ctl->privData;
> >  
> > +    goto fallback;
> 
> Obvious NACK since you did not make the patch look as a reproducer case
> to prove the code changes work.
> 
> I suggest adding a all-caps warning to the subject and NOT adding a
> Sign-off for reproducers not meant to be pushed.

Eh, nothing in this series is really meant to be pushed :)

But yours is indeed very good advice in general, so I'll keep it in
mind for future patches. Thanks!

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Oct 02, 2018 at 16:43:04 +0200, Andrea Bolognani wrote:
> On Tue, 2018-10-02 at 16:30 +0200, Peter Krempa wrote:
> > On Tue, Oct 02, 2018 at 16:14:46 +0200, Andrea Bolognani wrote:
> > > Instead of trying newer APIs first and only falling back to
> > > deprecated ones if those are not available, go straight for
> > > the older APIs.
> > > 
> > > Running 'virsh list' will now cause warnings to be displayed.
> > > 
> > > Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> > > ---
> > >  tools/virsh-domain-monitor.c | 2 ++
> > >  1 file changed, 2 insertions(+)

[...]

> > I suggest adding a all-caps warning to the subject and NOT adding a
> > Sign-off for reproducers not meant to be pushed.
> 
> Eh, nothing in this series is really meant to be pushed :)

Such approach without any obvious markers is only warranted on 1st of
April.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, 2018-10-02 at 17:26 +0200, Peter Krempa wrote:
> On Tue, Oct 02, 2018 at 16:43:04 +0200, Andrea Bolognani wrote:
[...]
> > Eh, nothing in this series is really meant to be pushed :)
> 
> Such approach without any obvious markers is only warranted on 1st of
> April.

I figured the series being marked as RFC, along with the cover letter
containing a "Known issues" section mentioning how the implementation
completely fails to work in one out of two sample scenarios, would be
enough to tip readers off.

How do you recommend I make it more obvious next time?

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list