Libguestfs - May 2015 - Re: [PATCH 00/10] virt-resize: add support for resizing MBR logical partitions

On 04/30/2015 06:51 PM, Chen, Hanxiao wrote:
> Hi, Rich
>
>> -----Original Message-----
>> From: Richard W.M. Jones [mailto:rjones@redhat.com]
>> Sent: Wednesday, April 29, 2015 7:04 PM
>> To: Chen, Hanxiao/陈 晗霄
>> Cc: libguestfs@redhat.com
>> Subject: Re: [Libguestfs] [PATCH 00/10] virt-resize: add support for
resizing MBR
>> logical partitions
>>
>> On Wed, Apr 29, 2015 at 09:19:24AM +0000, Chen, Hanxiao wrote:
>>> commit 6e989fd709fea37430398cbb7c01c52cd44bd1ac already did this:
>>> we had p_mbr_p_type : partition_type in type partition.
>>> Like what MBR/EBR did, this patchset use `partition_type`:
>>>
>>> /dev/sda3:
>>> 	partition data: 3 210763776-420478975 (209715200 bytes)
>>> 	bootable: false
>>> 	partition ID: 0x5
>>> 	content: extended partition
>>> 	label: (none)
>>> 	GUID: (none)
>>> 	*partition type: extended*
>>> /dev/sda5:
>>> 	partition data: 5 211812352-222298111 (10485760 bytes)
>>> 	bootable: false
>>> 	partition ID: 0x83
>>> 	content: unknown data
>>> 	label: (none)
>>> 	GUID: (none)
>>> 	*partition type: logical*
>>>
>>> As we could only had one extended partition,
>>> all logical partitions belong to that extended partition,
>>> so current type partition of virt-resize can reflect
logical/extended partition
>> model.
>>
>> The specific problem is that the extended partition (eg /dev/sda3)
>> contains the logical partition (/dev/sda5).  Having a flat list
>> doesn't express that at all.
> We had at most 4 PTE in MBR, in PTE, a partition type is on +04h, 
> value 05h or 0fh shows it's an extended partition.
> From extended partition's EBR, we got logical partitions.
>
> sda1-- sda2 -- sda3 -- sda4
>                    ^
>                    |
>                   sda5 -- sda6
>
> I think we do not need to show this specially,
> just use the original design:
> logical partitions existed in extended partition, an implicit relations.
>
> So a flat list can do this:
> We mark each members, then a tree view above could be rebuilt precisely
> from plat list.
Hi, Rich

Any comments on this?

Regards,
- Chen
>
>
> In this patchset, logical partitions are not filter out, it did:
>
> 1) partitions list: sda1 sda2 sda3 sda5
> every member of partitions got a member `partition type`
>
> 2) calculate surplus
>
> 3) then filter out logical partitions list:
> logical_partitions: sda5
> partitions: sda1 sda2 sda3
> do something to it (calculate_target_partitions...)
>
> 4) part_add to 'non-logical' partitions and logical partitions,
> then concrete them together for the original partition list.
>
> 5) copy_partition (ignore extended partitions, for we had logical partition
> in partitions list, we copy data from there)
>
>> I agree it is very hard to express this, because extended partitions
>> just work so differently from how any sensible partition scheme should
>> work (because they are a big hack).  But perhaps:
>>
>>  - flat list of primary partitions (as now, the global
'partitions')
> members of partitions list whose partition type is 'primary'
>>  - one MBR extended partition (as a new global variable)
> member of partitions list whose partition type is 'extended'.
>
>>  - flat list of logical partitions
> members of partitions list whose partition type is 'logical'.
>
> So I think we do not need 2 list and a global variable,
> if we want to know something, just do a List.filter.
>
>> would do it?  It still has the problem that the extended partition is
>> a bit like a primary partition, yet isn't in the partitions list.
> For the point of PTE's view, it does.
> PS: In my patch, it's in partitions list :)
>
> Regards
> - Chen
>
>>>> It's very hard to understand what changes are being made. 
The commits
>>>> are not grouped in a way that makes them easily understandable
to me.
>>> I'll try to regroup them and make the patchset easy to review.
>> Thanks,
>>
>> 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

On Fri, May 08, 2015 at 10:26:22PM +0800, Chen Hanxiao
wrote:
> 
> On 04/30/2015 06:51 PM, Chen, Hanxiao wrote:
> > Hi, Rich
> >
> >> -----Original Message-----
> >> From: Richard W.M. Jones [mailto:rjones@redhat.com]
> >> Sent: Wednesday, April 29, 2015 7:04 PM
> >> To: Chen, Hanxiao/陈 晗霄
> >> Cc: libguestfs@redhat.com
> >> Subject: Re: [Libguestfs] [PATCH 00/10] virt-resize: add support
for resizing MBR
> >> logical partitions
> >>
> >> On Wed, Apr 29, 2015 at 09:19:24AM +0000, Chen, Hanxiao wrote:
> >>> commit 6e989fd709fea37430398cbb7c01c52cd44bd1ac already did
this:
> >>> we had p_mbr_p_type : partition_type in type partition.
> >>> Like what MBR/EBR did, this patchset use `partition_type`:
> >>>
> >>> /dev/sda3:
> >>> 	partition data: 3 210763776-420478975 (209715200 bytes)
> >>> 	bootable: false
> >>> 	partition ID: 0x5
> >>> 	content: extended partition
> >>> 	label: (none)
> >>> 	GUID: (none)
> >>> 	*partition type: extended*
> >>> /dev/sda5:
> >>> 	partition data: 5 211812352-222298111 (10485760 bytes)
> >>> 	bootable: false
> >>> 	partition ID: 0x83
> >>> 	content: unknown data
> >>> 	label: (none)
> >>> 	GUID: (none)
> >>> 	*partition type: logical*
> >>>
> >>> As we could only had one extended partition,
> >>> all logical partitions belong to that extended partition,
> >>> so current type partition of virt-resize can reflect
logical/extended partition
> >> model.
> >>
> >> The specific problem is that the extended partition (eg /dev/sda3)
> >> contains the logical partition (/dev/sda5).  Having a flat list
> >> doesn't express that at all.
> > We had at most 4 PTE in MBR, in PTE, a partition type is on +04h, 
> > value 05h or 0fh shows it's an extended partition.
> > From extended partition's EBR, we got logical partitions.
> >
> > sda1-- sda2 -- sda3 -- sda4
> >                    ^
> >                    |
> >                   sda5 -- sda6
> >
> > I think we do not need to show this specially,
> > just use the original design:
> > logical partitions existed in extended partition, an implicit
relations.
> >
> > So a flat list can do this:
> > We mark each members, then a tree view above could be rebuilt
precisely
> > from plat list.
> Hi, Rich
> 
> Any comments on this?
As I said in my original email, my opinion is still that the code
would be clearer if it expressed the peculiar structure of MBR
extended/logical partitions directly in the structures.

There's really a high hurdle for these changes.  They must be:

 - easy to review

 - absolutely not break existing uses of virt-resize

We're nowhere near there at the moment.  There's no good testing,
there are no patches for review which look sound.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html