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

[Richard W.M. Jones]

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 at 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.
> +
> + at section Related binaries
> +
> + at 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
> +
> + at example{qemu-nbd -P 1 -f qcow2 file.qcow2}
> +
> +can be rewritten as:
> +
> + at 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
> + at 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 at 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