Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper
On Wed, Mar 04, 2020 at 03:13:41PM +0100, Peter Krempa wrote: > On Wed, Mar 04, 2020 at 15:01:07 +0100, Pavel Mores wrote: > > On Wed, Mar 04, 2020 at 02:53:53PM +0100, Peter Krempa wrote: > > > On Wed, Mar 04, 2020 at 14:41:09 +0100, Pavel Mores wrote: > > > > 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 > > > > > > --- > > > > > > include/libvirt/libvirt-domain.h | 4 ++-- > > > > > > src/libvirt-domain.c | 5 +++-- > > > > > > tools/virsh-domain.c | 14 +++--- > > > > > > 3 files changed, 16 insertions(+), 7 deletions(-) > > [...] > > > > This is obviously a lot of work, thus we need to decide whether adding > > > an old-school API is worth it in the inerim. Are there any real users > > > who would benefit from the new pull semantics? blockpull is around for a > > > long time already, but it seems that commit is favoured. > > > > > > If there is no real demand though I'd probably prefer if we don't add > > > any more block job APIs any more. > > > > I'm not aware of any real demand for this, however as I stated in the cover > > letter I believe I need full blockpull to deal with the bug I'm actually > > working on, which is full support for external snapshots in snapshot-delete. > > Deleting/reverting external snapshots needs to be done internally under > the hood of virDomainRevertToSnapshot/virDomainSnapshotDelete so if you > require use of the 'block-stream' command to an intermediate layer you > don't actually need to expose it via virDomainBlockPull/Rebase to take > advantage of it. That's true. I think this basically means I should drop patches 3 a 4 from this series (I'll keep them locally as they give me a reasonably easy way to test my changes), right? > For reversion of external snapshots you'll probably need a new API > anyways as you'll need to be able to specify a new set of disk images to > hold the writes. Okay, I'll deal with that in due time, I guess when I'm back to working on snapshot-delete. pvl
Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper
On Wed, Mar 04, 2020 at 15:01:07 +0100, Pavel Mores wrote: > On Wed, Mar 04, 2020 at 02:53:53PM +0100, Peter Krempa wrote: > > On Wed, Mar 04, 2020 at 14:41:09 +0100, Pavel Mores wrote: > > > 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 > > > > > --- > > > > > include/libvirt/libvirt-domain.h | 4 ++-- > > > > > src/libvirt-domain.c | 5 +++-- > > > > > tools/virsh-domain.c | 14 +++--- > > > > > 3 files changed, 16 insertions(+), 7 deletions(-) [...] > > This is obviously a lot of work, thus we need to decide whether adding > > an old-school API is worth it in the inerim. Are there any real users > > who would benefit from the new pull semantics? blockpull is around for a > > long time already, but it seems that commit is favoured. > > > > If there is no real demand though I'd probably prefer if we don't add > > any more block job APIs any more. > > I'm not aware of any real demand for this, however as I stated in the cover > letter I believe I need full blockpull to deal with the bug I'm actually > working on, which is full support for external snapshots in snapshot-delete. Deleting/reverting external snapshots needs to be done internally under the hood of virDomainRevertToSnapshot/virDomainSnapshotDelete so if you require use of the 'block-stream' command to an intermediate layer you don't actually need to expose it via virDomainBlockPull/Rebase to take advantage of it. For reversion of external snapshots you'll probably need a new API anyways as you'll need to be able to specify a new set of disk images to hold the writes.
Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper
On Wed, Mar 04, 2020 at 02:53:53PM +0100, Peter Krempa wrote:
> On Wed, Mar 04, 2020 at 14:41:09 +0100, Pavel Mores wrote:
> > 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
> > > > ---
> > > > include/libvirt/libvirt-domain.h | 4 ++--
> > > > src/libvirt-domain.c | 5 +++--
> > > > tools/virsh-domain.c | 14 +++---
> > > > 3 files changed, 16 insertions(+), 7 deletions(-)
>
> [...]
>
> > > > @@ -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?
>
> If we are going to implement new APIs for blockjobs I'd prefer we take a
> step back and re-design the (block)-job APIs rather than add yet another
> oldschool one.
>
> Similarly to our criticisms of qemu's apis which didn't preserve the job
> state after the job finished, which is now fixed I've heard exactly the
> same requests from at least oVirt developers requesting we do the same
> thing.
>
> The new API(s) should thus return a job object (virDomainJob?) similarly
> to how the domain lookup APIs return them. This new object then can be
> used to refer to the job even after it has finished. Similarly to qemu
> we should also introduce provisions to auto-dismiss the job if the
> management APP doesn't wish to have to deal with it (and that probably
> should be default behaviour given how we approached it previously).
>
> This will then obviously require support APIs for looking up/listing
> jobs, getting the state or configuration etc.
>
> This is obviously a lot of work, thus we need to decide whether adding
> an old-school API is worth it in the inerim. Are there any real users
> who would benefit from the new pull semantics? blockpull is around for a
> long time already, but it seems that commit is favoured.
>
> If there is no real demand though I'd probably prefer if we don't add
> any more block job APIs any more.
I'm not aware of any real demand for this, however as I stated in the cover
letter I believe I need full blockpull to deal with the bug I'm actually
working on, which is full support for external snapshots in snapshot-delete.
Considering what you said, the "dubious trick" I mentioned in the other branch
of this thread might be a bearable interim solution...
pvl
Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper
On Wed, Mar 04, 2020 at 14:55:30 +0100, Pavel Mores wrote:
> On Wed, Mar 04, 2020 at 02:41:09PM +0100, Pavel Mores wrote:
> > 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
> > > > ---
> > > > 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.
>
> Actually, on second thought, it might not be as dubious as I first thought.
> Certainly not as clean as just adding a parameter in general but depending on
> what the cost of adding a new API would be, reusing the existing 'base' param
> might be workable.
That's gross. Please don't do that.
>
> Also, how about the RPC change, is that acceptable?
No, that's on same par as API.
>
> Thanks,
>
> pvl
>
Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper
On Wed, Mar 04, 2020 at 02:41:09PM +0100, Pavel Mores wrote:
> 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
> > > ---
> > > 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.
Actually, on second thought, it might not be as dubious as I first thought.
Certainly not as clean as just adding a parameter in general but depending on
what the cost of adding a new API would be, reusing the existing 'base' param
might be workable.
Also, how about the RPC change, is that acceptable?
Thanks,
pvl
Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper
On Wed, Mar 04, 2020 at 14:41:09 +0100, Pavel Mores wrote:
> 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
> > > ---
> > > include/libvirt/libvirt-domain.h | 4 ++--
> > > src/libvirt-domain.c | 5 +++--
> > > tools/virsh-domain.c | 14 +++---
> > > 3 files changed, 16 insertions(+), 7 deletions(-)
[...]
> > > @@ -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?
If we are going to implement new APIs for blockjobs I'd prefer we take a
step back and re-design the (block)-job APIs rather than add yet another
oldschool one.
Similarly to our criticisms of qemu's apis which didn't preserve the job
state after the job finished, which is now fixed I've heard exactly the
same requests from at least oVirt developers requesting we do the same
thing.
The new API(s) should thus return a job object (virDomainJob?) similarly
to how the domain lookup APIs return them. This new object then can be
used to refer to the job even after it has finished. Similarly to qemu
we should also introduce provisions to auto-dismiss the job if the
management APP doesn't wish to have to deal with it (and that probably
should be default behaviour given how we approached it previously).
This will then obviously require support APIs for looking up/listing
jobs, getting the state or configuration etc.
This is obviously a lot of work, thus we need to decide whether adding
an old-school API is worth it in the inerim. Are there any real users
who would benefit from the new pull semantics? blockpull is around for a
long time already, but it seems that commit is favoured.
If there is no real demand though I'd probably prefer if we don't add
any more block job APIs any more.
Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper
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
> > ---
> > 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
Re: [libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper
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
> ---
> 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.
[libvirt PATCH 3/6] qemu: block: blockpull param 'top' support in virsh proper
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
---
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;
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 9d0f7d68d2..47ccdd1b94 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -2436,7 +2436,7 @@ cmdBlockcopy(vshControl *ctl, const vshCmd *cmd)
if (bytes)
flags |= VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES;
-if (virDomainBlockRebase(dom, path, dest, bandwidth, flags) < 0)
+if (virDomainBlockRebase(dom, path, dest, NULL, bandwidth, flags) < 0)
goto cleanup;
}
@@ -2765,6 +2765,10 @@ static const vshCmdOptDef opts_blockpull[] = {
.type = VSH_OT_STRING,
.help = N_("path of backing file in chain for a partial pull")
},
+{.name = "top",
+ .type = VSH_OT_STRING,
+ .help = N_("path of top backing file in chain for a partial pull")
+},
{.name = "wait",
.type = VSH_OT_BOOL,
.help = N_("wait for job to finish")
@@ -2804,6 +2808,7 @@ cmdBlockpull(vshControl *ctl, const vshCmd *cmd)
int timeout = 0;
const char *path = NULL;
const char *base = NULL;
+const char *top = NULL;
unsigned long bandwidth = 0;
unsigned int flags = 0;
virshBlockJobWaitDataPtr bjWait = NULL;
@@ -2817,6 +2822,9 @@ cmdBlockpull(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0)
return false;
+if (vshCommandOptStringReq(ctl, cmd, "top", &top) < 0)
+return false;
+
if (vshBlockJobOptionBandwidth(ctl, cmd, bytes, &bandwidth) < 0)
return false;
@@ -2834,11 +2842,11 @@ cmdBlockpull(vshControl *ctl, const vshCmd *cmd)
verbose, timeout, async)))
goto cleanup;
-if (base || flags) {
+if (base || top || flags) {
if (bytes)
flags |= VIR_DOMAIN_BLOCK_REBASE_BANDWIDTH_BYTES;
-if (virDomainBlockRebase(dom, path, base, bandwidth, flags) < 0)
+if (virDomainBlockRebase(dom, path, base, top, bandwidth, flags) < 0)
goto cleanup;
} else {
if (bytes)
--
2.24.1
