[libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper

[Pavel Mores]

On Wed, Mar 04, 2020 at 01:00:59PM +0100, Peter Krempa wrote:
> On Wed, Mar 04, 2020 at 11:12:37 +0100, Pavel Mores wrote:
> > A new command-line option --top was added to virsh's blockpull command.
> > Similar to how --base is handled, in presence of --top the operation is
> > implemented internally as a rebase.  To that end, a corresponding new 'top'
> > parameter was added to virDomainBlockRebase().
> > 
> > Signed-off-by: Pavel Mores <pmores at redhat.com>
> > ---
> >  include/libvirt/libvirt-domain.h |  4 ++--
> >  src/libvirt-domain.c             |  5 +++--
> >  tools/virsh-domain.c             | 14 +++++++++++---
> >  3 files changed, 16 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> > index b440818ec2..069d7f98ec 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -2560,8 +2560,8 @@ typedef enum {
> >  } virDomainBlockRebaseFlags;
> >  
> >  int virDomainBlockRebase(virDomainPtr dom, const char *disk,
> > -                         const char *base, unsigned long bandwidth,
> > -                         unsigned int flags);
> > +                         const char *base, const char *top,
> > +                         unsigned long bandwidth, unsigned int flags);
> >  
> >  /**
> >   * virDomainBlockCopyFlags:
> > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> > index 65813b68cc..1f9d1b5b84 100644
> > --- a/src/libvirt-domain.c
> > +++ b/src/libvirt-domain.c
> > @@ -10150,6 +10150,7 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,
> >   * @disk: path to the block device, or device shorthand
> >   * @base: path to backing file to keep, or device shorthand,
> >   *        or NULL for no backing file
> > + * @top: path to top file, or device shorthand, or NULL for no top
> >   * @bandwidth: (optional) specify bandwidth limit; flags determine the unit
> >   * @flags: bitwise-OR of virDomainBlockRebaseFlags
> >   *
> > @@ -10257,8 +10258,8 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,
> >   */
> >  int
> >  virDomainBlockRebase(virDomainPtr dom, const char *disk,
> > -                     const char *base, unsigned long bandwidth,
> > -                     unsigned int flags)
> > +                     const char *base, const char *top,
> > +                     unsigned long bandwidth, unsigned int flags)
> >  {
> >      virConnectPtr conn;
> 
> So this is one thing we can't do. This modifies the public API of
> libvirt and would cause software which is already compiled to pass wrong
> arguments to this function.
> 
> If the semantics can't be mapped to existing arguments of this function
> we'll need to add a new function in the first place.

Yes.  Seeing as the function takes just a bunch of primitive data types and a
virDomainPtr, I'm afraid I don't see much room for not changing the signature,
apart from arguably dubious tricks like stuffing both base and top to the
existing 'base' parameter and parsing the individual values out of it in the
function body.

Any convention or suggestion to help pick a good name for the new function?

Thanks,

	pvl