[libvirt] [PATCH] qemu-nbd: Deprecate qemu-nbd --partition

Eric Blake posted 1 patch 5 years, 2 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
qemu-deprecated.texi | 27 +++++++++++++++++++++++++++
qemu-nbd.texi        |  6 ++++--
qemu-nbd.c           |  2 ++
3 files changed, 33 insertions(+), 2 deletions(-)
[libvirt] [PATCH] qemu-nbd: Deprecate qemu-nbd --partition
Posted by Eric Blake 5 years, 2 months ago
The existing qemu-nbd --partition code claims to handle logical
partitions up to 8, since its introduction in 2008 (commit 7a5ca86).
However, the implementation is bogus (actual MBR logical partitions
form a sort of linked list, with one partition per extended table
entry, rather than four logical partitions in a single extended
table), making the code unlikely to work for anything beyond -P5 on
actual guest images. What's more, the code does not support GPT
partitions, which are becoming more popular, and maintaining device
subsetting in both NBD and the raw device is unnecessary maintenance
burden.  And nbdkit has just added code to properly handle an
arbitrary number of MBR partitions, along with its existing code
for handling GPT partitions.

Note that obtaining the offsets of a partition can be learned by
using 'qemu-nbd -c /dev/nbd0 file.qcow2 && sfdisk --dump /dev/nbd0',
but by the time you've done that, you might as well just mount
/dev/nbd0p1 that the kernel creates for you.

Start the clock on the deprecation cycle, with an example of how
to write device subsetting without using -P.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qemu-deprecated.texi | 27 +++++++++++++++++++++++++++
 qemu-nbd.texi        |  6 ++++--
 qemu-nbd.c           |  2 ++
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 219206a836f..12f8b30943f 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -175,3 +175,30 @@ The above, converted to the current supported format:
 @subsubsection "irq": "" (since 3.0.0)

 The ``irq'' property is obsoleted.
+
+@section Related binaries
+
+@subsection qemu-nbd --partition (since 4.0.0)
+
+The ``qemu-nbd --partition $digit'' code (also spelled @option{-P})
+can only handle MBR partitions, and does not correctly handle logical
+partitions beyond partition 5.  If you know the relative position of
+the partition (perhaps by using @code{sfdisk} or similar, either in
+the guest or when mapping the entire device through /dev/nbd0 in the
+host), you can achieve the effect of exporting just that subset of the
+disk by use of the @option{--image-opts} option with a raw blockdev
+using the @code{offset} and @code{size} parameters layered on top of
+any other existing blockdev.
+
+For example, if partition 1 is 100MiB starting at 1MiB, the old command
+
+@example{qemu-nbd -P 1 -f qcow2 file.qcow2}
+
+can be rewritten as:
+
+@example{qemu-nbd --image-opts driver=raw,offset=1M,size=100M,file.driver=qcow2,file.backing.driver=file,file.backing.filename=file.qcow2}
+
+Alternatively, the @code{nbdkit} project provides a more powerful
+partition filter on top of its nbd plugin, which can be used to select
+an arbitrary MBR or GPT partition on top of any other full-image NBD
+export.
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 386bece4680..d0c51828149 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -56,8 +56,10 @@ auto-detecting.
 @item -r, --read-only
 Export the disk as read-only.
 @item -P, --partition=@var{num}
-Only expose MBR partition @var{num}.  Understands physical partitions
-1-4 and logical partitions 5-8.
+Deprecated: Only expose MBR partition @var{num}.  Understands physical
+partitions 1-4 and logical partition 5. New code should instead use
+@option{--image-opts} with the raw driver wrapping a subset of the
+original image.
 @item -B, --bitmap=@var{name}
 If @var{filename} has a qcow2 persistent bitmap @var{name}, expose
 that bitmap via the ``qemu:dirty-bitmap:@var{name}'' context
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 1f7b2a03f5d..00c07fd27ea 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -787,6 +787,8 @@ int main(int argc, char **argv)
             flags &= ~BDRV_O_RDWR;
             break;
         case 'P':
+            warn_report("The '-P' option is deprecated; use --image-opts with "
+                        "a raw device wrapper for subset exports instead");
             if (qemu_strtoi(optarg, NULL, 0, &partition) < 0 ||
                 partition < 1 || partition > 8) {
                 error_report("Invalid partition '%s'", optarg);
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu-nbd: Deprecate qemu-nbd --partition
Posted by Richard W.M. Jones 5 years, 2 months ago
On Wed, Jan 23, 2019 at 03:19:53PM -0600, Eric Blake wrote:
> The existing qemu-nbd --partition code claims to handle logical
> partitions up to 8, since its introduction in 2008 (commit 7a5ca86).
> However, the implementation is bogus (actual MBR logical partitions
> form a sort of linked list, with one partition per extended table
> entry, rather than four logical partitions in a single extended
> table), making the code unlikely to work for anything beyond -P5 on
> actual guest images. What's more, the code does not support GPT
> partitions, which are becoming more popular, and maintaining device
> subsetting in both NBD and the raw device is unnecessary maintenance
> burden.  And nbdkit has just added code to properly handle an
> arbitrary number of MBR partitions, along with its existing code
> for handling GPT partitions.
> 
> Note that obtaining the offsets of a partition can be learned by
> using 'qemu-nbd -c /dev/nbd0 file.qcow2 && sfdisk --dump /dev/nbd0',
> but by the time you've done that, you might as well just mount
> /dev/nbd0p1 that the kernel creates for you.
> 
> Start the clock on the deprecation cycle, with an example of how
> to write device subsetting without using -P.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  qemu-deprecated.texi | 27 +++++++++++++++++++++++++++
>  qemu-nbd.texi        |  6 ++++--
>  qemu-nbd.c           |  2 ++
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 219206a836f..12f8b30943f 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -175,3 +175,30 @@ The above, converted to the current supported format:
>  @subsubsection "irq": "" (since 3.0.0)
> 
>  The ``irq'' property is obsoleted.
> +
> +@section Related binaries
> +
> +@subsection qemu-nbd --partition (since 4.0.0)
> +
> +The ``qemu-nbd --partition $digit'' code (also spelled @option{-P})
> +can only handle MBR partitions, and does not correctly handle logical
> +partitions beyond partition 5.  If you know the relative position of
> +the partition (perhaps by using @code{sfdisk} or similar, either in
> +the guest or when mapping the entire device through /dev/nbd0 in the
> +host), you can achieve the effect of exporting just that subset of the
> +disk by use of the @option{--image-opts} option with a raw blockdev
> +using the @code{offset} and @code{size} parameters layered on top of
> +any other existing blockdev.
> +
> +For example, if partition 1 is 100MiB starting at 1MiB, the old command
> +
> +@example{qemu-nbd -P 1 -f qcow2 file.qcow2}
> +
> +can be rewritten as:
> +
> +@example{qemu-nbd --image-opts driver=raw,offset=1M,size=100M,file.driver=qcow2,file.backing.driver=file,file.backing.filename=file.qcow2}
> +
> +Alternatively, the @code{nbdkit} project provides a more powerful
> +partition filter on top of its nbd plugin, which can be used to select
> +an arbitrary MBR or GPT partition on top of any other full-image NBD
> +export.

You might want to add the actual command here.  Unfortunately nbdkit
cannot read qcow2 files meaning (as you note already) that you have to
forward the connection through the nbdkit-nbd-plugin to qemu-nbd.
This worked for me:

  qemu-nbd -t -k /tmp/sock -f qcow2 file.qcow2 &
  nbdkit -f --filter=partition nbd socket=/tmp/sock partition=1 &

If you drop the requirement to demonstrate this with qcow2 then the
command would be just this:

  nbdkit --filter=partition file disk.raw partition=1

> diff --git a/qemu-nbd.texi b/qemu-nbd.texi
> index 386bece4680..d0c51828149 100644
> --- a/qemu-nbd.texi
> +++ b/qemu-nbd.texi
> @@ -56,8 +56,10 @@ auto-detecting.
>  @item -r, --read-only
>  Export the disk as read-only.
>  @item -P, --partition=@var{num}
> -Only expose MBR partition @var{num}.  Understands physical partitions
> -1-4 and logical partitions 5-8.
> +Deprecated: Only expose MBR partition @var{num}.  Understands physical
> +partitions 1-4 and logical partition 5. New code should instead use
> +@option{--image-opts} with the raw driver wrapping a subset of the
> +original image.
>  @item -B, --bitmap=@var{name}
>  If @var{filename} has a qcow2 persistent bitmap @var{name}, expose
>  that bitmap via the ``qemu:dirty-bitmap:@var{name}'' context
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 1f7b2a03f5d..00c07fd27ea 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -787,6 +787,8 @@ int main(int argc, char **argv)
>              flags &= ~BDRV_O_RDWR;
>              break;
>          case 'P':
> +            warn_report("The '-P' option is deprecated; use --image-opts with "
> +                        "a raw device wrapper for subset exports instead");
>              if (qemu_strtoi(optarg, NULL, 0, &partition) < 0 ||
>                  partition < 1 || partition > 8) {
>                  error_report("Invalid partition '%s'", optarg);

But this is basically fine, so:

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu-nbd: Deprecate qemu-nbd --partition
Posted by Eric Blake 5 years, 2 months ago
On 1/23/19 3:55 PM, Richard W.M. Jones wrote:
> On Wed, Jan 23, 2019 at 03:19:53PM -0600, Eric Blake wrote:
>> The existing qemu-nbd --partition code claims to handle logical
>> partitions up to 8, since its introduction in 2008 (commit 7a5ca86).
>> However, the implementation is bogus (actual MBR logical partitions
>> form a sort of linked list, with one partition per extended table
>> entry, rather than four logical partitions in a single extended
>> table), making the code unlikely to work for anything beyond -P5 on
>> actual guest images. What's more, the code does not support GPT
>> partitions, which are becoming more popular, and maintaining device
>> subsetting in both NBD and the raw device is unnecessary maintenance
>> burden.  And nbdkit has just added code to properly handle an
>> arbitrary number of MBR partitions, along with its existing code
>> for handling GPT partitions.
>>
>> Note that obtaining the offsets of a partition can be learned by
>> using 'qemu-nbd -c /dev/nbd0 file.qcow2 && sfdisk --dump /dev/nbd0',
>> but by the time you've done that, you might as well just mount
>> /dev/nbd0p1 that the kernel creates for you.
>>
>> Start the clock on the deprecation cycle, with an example of how
>> to write device subsetting without using -P.
>>

>> +For example, if partition 1 is 100MiB starting at 1MiB, the old command
>> +
>> +@example{qemu-nbd -P 1 -f qcow2 file.qcow2}
>> +
>> +can be rewritten as:
>> +
>> +@example{qemu-nbd --image-opts driver=raw,offset=1M,size=100M,file.driver=qcow2,file.backing.driver=file,file.backing.filename=file.qcow2}
>> +
>> +Alternatively, the @code{nbdkit} project provides a more powerful
>> +partition filter on top of its nbd plugin, which can be used to select
>> +an arbitrary MBR or GPT partition on top of any other full-image NBD
>> +export.
> 
> You might want to add the actual command here.

Good idea - as long as we are deprecating something, telling the user
how to get the same functionality (in this case, user-space partition
detection, without involving /dev/nbd) is worth the extra effort.

>  Unfortunately nbdkit
> cannot read qcow2 files meaning (as you note already) that you have to
> forward the connection through the nbdkit-nbd-plugin to qemu-nbd.
> This worked for me:
> 
>   qemu-nbd -t -k /tmp/sock -f qcow2 file.qcow2 &
>   nbdkit -f --filter=partition nbd socket=/tmp/sock partition=1 &

Is the -f necessary? Otherwise, yes, this looks reasonable.  I'll add it
for v2.

> If you drop the requirement to demonstrate this with qcow2 then the
> command would be just this:
> 
>   nbdkit --filter=partition file disk.raw partition=1
> 


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu-nbd: Deprecate qemu-nbd --partition
Posted by Richard W.M. Jones 5 years, 2 months ago
On Wed, Jan 23, 2019 at 04:18:16PM -0600, Eric Blake wrote:
> On 1/23/19 3:55 PM, Richard W.M. Jones wrote:
> > On Wed, Jan 23, 2019 at 03:19:53PM -0600, Eric Blake wrote:
> >> The existing qemu-nbd --partition code claims to handle logical
> >> partitions up to 8, since its introduction in 2008 (commit 7a5ca86).
> >> However, the implementation is bogus (actual MBR logical partitions
> >> form a sort of linked list, with one partition per extended table
> >> entry, rather than four logical partitions in a single extended
> >> table), making the code unlikely to work for anything beyond -P5 on
> >> actual guest images. What's more, the code does not support GPT
> >> partitions, which are becoming more popular, and maintaining device
> >> subsetting in both NBD and the raw device is unnecessary maintenance
> >> burden.  And nbdkit has just added code to properly handle an
> >> arbitrary number of MBR partitions, along with its existing code
> >> for handling GPT partitions.
> >>
> >> Note that obtaining the offsets of a partition can be learned by
> >> using 'qemu-nbd -c /dev/nbd0 file.qcow2 && sfdisk --dump /dev/nbd0',
> >> but by the time you've done that, you might as well just mount
> >> /dev/nbd0p1 that the kernel creates for you.
> >>
> >> Start the clock on the deprecation cycle, with an example of how
> >> to write device subsetting without using -P.
> >>
> 
> >> +For example, if partition 1 is 100MiB starting at 1MiB, the old command
> >> +
> >> +@example{qemu-nbd -P 1 -f qcow2 file.qcow2}
> >> +
> >> +can be rewritten as:
> >> +
> >> +@example{qemu-nbd --image-opts driver=raw,offset=1M,size=100M,file.driver=qcow2,file.backing.driver=file,file.backing.filename=file.qcow2}
> >> +
> >> +Alternatively, the @code{nbdkit} project provides a more powerful
> >> +partition filter on top of its nbd plugin, which can be used to select
> >> +an arbitrary MBR or GPT partition on top of any other full-image NBD
> >> +export.
> > 
> > You might want to add the actual command here.
> 
> Good idea - as long as we are deprecating something, telling the user
> how to get the same functionality (in this case, user-space partition
> detection, without involving /dev/nbd) is worth the extra effort.
> 
> >  Unfortunately nbdkit
> > cannot read qcow2 files meaning (as you note already) that you have to
> > forward the connection through the nbdkit-nbd-plugin to qemu-nbd.
> > This worked for me:
> > 
> >   qemu-nbd -t -k /tmp/sock -f qcow2 file.qcow2 &
> >   nbdkit -f --filter=partition nbd socket=/tmp/sock partition=1 &
> 
> Is the -f necessary? Otherwise, yes, this looks reasonable.  I'll add it
> for v2.

It's not necessary, but it makes nbdkit behave the same way with
respect to remaining in the foreground as qemu-nbd.

Rich.

> > If you drop the requirement to demonstrate this with qcow2 then the
> > command would be just this:
> > 
> >   nbdkit --filter=partition file disk.raw partition=1
> > 
> 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 




-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu-nbd: Deprecate qemu-nbd --partition
Posted by Peter Krempa 5 years, 2 months ago
On Wed, Jan 23, 2019 at 15:19:53 -0600, Eric Blake wrote:
> The existing qemu-nbd --partition code claims to handle logical
> partitions up to 8, since its introduction in 2008 (commit 7a5ca86).
> However, the implementation is bogus (actual MBR logical partitions
> form a sort of linked list, with one partition per extended table
> entry, rather than four logical partitions in a single extended
> table), making the code unlikely to work for anything beyond -P5 on
> actual guest images. What's more, the code does not support GPT
> partitions, which are becoming more popular, and maintaining device
> subsetting in both NBD and the raw device is unnecessary maintenance
> burden.  And nbdkit has just added code to properly handle an
> arbitrary number of MBR partitions, along with its existing code
> for handling GPT partitions.
> 
> Note that obtaining the offsets of a partition can be learned by
> using 'qemu-nbd -c /dev/nbd0 file.qcow2 && sfdisk --dump /dev/nbd0',
> but by the time you've done that, you might as well just mount
> /dev/nbd0p1 that the kernel creates for you.
> 
> Start the clock on the deprecation cycle, with an example of how
> to write device subsetting without using -P.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  qemu-deprecated.texi | 27 +++++++++++++++++++++++++++
>  qemu-nbd.texi        |  6 ++++--
>  qemu-nbd.c           |  2 ++
>  3 files changed, 33 insertions(+), 2 deletions(-)

Libvirt does not use qemu-nbd in any way so it's okay from our POV.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list