Libguestfs - Jan 2019 - Re: [PATCH nbdkit v2 1/4] partitioning plugin: Support MBR logical partitions.

On 1/21/19 12:15 PM, Richard W.M. Jones wrote:
> ---
>  .../nbdkit-partitioning-plugin.pod            |  29 ++--
>  plugins/partitioning/virtual-disk.h           |  12 +-
>  plugins/partitioning/partition-mbr.c          | 132 +++++++++++++++---
>  plugins/partitioning/partitioning.c           |  28 ++--
>  plugins/partitioning/virtual-disk.c           |  42 +++++-
>  tests/Makefile.am                             |   4 +-
>  tests/test-partitioning5.sh                   |  96 +++++++++++++
>  7 files changed, 281 insertions(+), 62 deletions(-)
LGTM

Should we, at some point, allow the plugin user to specify the
advertised filesystem type and/or the boot flag for various partitions
(and not just that all file names presented are exposed as non-bootable
partitions of the same hard-coded type)?  But that's a separate addition.

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

On Tue, Jan 22, 2019 at 02:57:33PM -0600, Eric Blake
wrote:
> On 1/21/19 12:15 PM, Richard W.M. Jones wrote:
> > ---
> >  .../nbdkit-partitioning-plugin.pod            |  29 ++--
> >  plugins/partitioning/virtual-disk.h           |  12 +-
> >  plugins/partitioning/partition-mbr.c          | 132
+++++++++++++++---
> >  plugins/partitioning/partitioning.c           |  28 ++--
> >  plugins/partitioning/virtual-disk.c           |  42 +++++-
> >  tests/Makefile.am                             |   4 +-
> >  tests/test-partitioning5.sh                   |  96 +++++++++++++
> >  7 files changed, 281 insertions(+), 62 deletions(-)
> 
> LGTM
> 
> Should we, at some point, allow the plugin user to specify the
> advertised filesystem type
Is this different from the mbr-id / type-guid parameters that we
already provide?
> and/or the boot flag
Yes this is missing and it would be a good idea to allow it to be
specified.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On 1/22/19 3:10 PM, Richard W.M. Jones wrote:
> On Tue, Jan 22, 2019 at 02:57:33PM -0600, Eric Blake wrote:
>> On 1/21/19 12:15 PM, Richard W.M. Jones wrote:
>>> ---
>>>  .../nbdkit-partitioning-plugin.pod            |  29 ++--
>>>  plugins/partitioning/virtual-disk.h           |  12 +-
>>>  plugins/partitioning/partition-mbr.c          | 132
+++++++++++++++---
>>>  plugins/partitioning/partitioning.c           |  28 ++--
>>>  plugins/partitioning/virtual-disk.c           |  42 +++++-
>>>  tests/Makefile.am                             |   4 +-
>>>  tests/test-partitioning5.sh                   |  96 +++++++++++++
>>>  7 files changed, 281 insertions(+), 62 deletions(-)
>>
>> LGTM
>>
>> Should we, at some point, allow the plugin user to specify the
>> advertised filesystem type
> 
> Is this different from the mbr-id / type-guid parameters that we
> already provide?
Aha - that's what I was missing. (Patch 2/4 used type-guid, but I think
it wanted to use mbr-id).
> 
>> and/or the boot flag
> 
> Yes this is missing and it would be a good idea to allow it to be
> specified.
> 
> Rich.
> 
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org