Bug 602997

Summary:	part-get-bootable gives wrong result with an unordered part layout
Product:	[Community] Virtualization Tools	Reporter:	Jinxin Zheng <jzheng>
Component:	libguestfs	Assignee:	Richard W.M. Jones <rjones>
Status:	CLOSED RAWHIDE	QA Contact:
Severity:	medium	Docs Contact:
Priority:	low
Version:	unspecified	CC:	mbooth, virt-maint
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:
Clones:	603000 (view as bug list)		Environment:
Last Closed:	2011-07-13 08:57:32 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	603000

Description Jinxin Zheng 2010-06-11 08:27:19 UTC

Description of problem:
The do_part_get_bootable code parses the output of parted in an assumption that the partition layout on the guest image is well ordered, which could be problematic where the assumption is false.

Version-Release number of selected component (if applicable):
libguestfs-1.2.7

How reproducible:
Always.

Steps to Reproduce:
run this script,

$ guestfish <<EOF
alloc testfs 100M
run
part-init /dev/sda mbr
#create an unordered layout
part-add /dev/sda p 1001 2000
part-add /dev/sda p 1 1000
#this places the number 2 part in front of the number 1 part
part-list /dev/sda
part-set-bootable /dev/sda 1 1
part-get-bootable /dev/sda 1
part-get-bootable /dev/sda 2
EOF

Actual results:
(last) script output is

false
true

Expected results:

true
false

Additional info:
Partition numbers can be unordered especially for the images manipulated with partition tools like parted, fdisk, sfdisk, ...

This is even worse with some absent partition. Look at the layout created with the following steps,

...
><fs> part-init /dev/sda mbr
><fs> part-add /dev/sda p 1 1000
><fs> part-add /dev/sda p 1001 2000
><fs> part-add /dev/sda p 2001 3000
><fs> part-del /dev/sda 2
><fs> part-list /dev/sda
[0] = {
  part_num: 1
  part_start: 512
  part_end: 512511
  part_size: 512000
}
[1] = {
  part_num: 3
  part_start: 1024512
  part_end: 1536511
  part_size: 512000
}

This creates a layout that the number 2 part is missing. If we want to get the bootable flag of the number 3 part...

><fs> part-get-bootable /dev/sda 3
libguestfs: error: unexpected end of file when reading from daemon
><fs> ping-daemon 
libguestfs: error: guestfs_ping_daemon: call launch before using this function

This is causing the daemon to die ...

Fortunately the fix should be simple. The parsing algorithm should check the partition number on every line of the output, which I believe should effectively solve the problem.

Comment 1 Richard W.M. Jones 2010-06-11 08:42:27 UTC

This is RHEL 5 or Fedora?  We use two different code paths
for two different parted outputs, although I suspect that
both code paths are wrong since both assume that parted
will return the partitions in order starting at 1 with no gaps.

Here is the code for do_part_get_bootable:

http://git.annexia.org/?p=libguestfs.git;a=blob;f=daemon/parted.c;hb=HEAD#l545

Looking at that file, it seems that only part-get-bootable
should be affected by this issue.

Comment 2 Jinxin Zheng 2010-06-11 08:47:11 UTC

(In reply to comment #1)
> This is RHEL 5 or Fedora?  We use two different code paths
> for two different parted outputs, although I suspect that
> both code paths are wrong since both assume that parted
> will return the partitions in order starting at 1 with no gaps.
> 
I tested under fedora12,RHEL5,RHEL6, all produced..
> Here is the code for do_part_get_bootable:
> 
> http://git.annexia.org/?p=libguestfs.git;a=blob;f=daemon/parted.c;hb=HEAD#l545
> 
> Looking at that file, it seems that only part-get-bootable
> should be affected by this issue.    

Yes I see too. Please examine more carefully than I if you can.

Comment 3 Richard W.M. Jones 2011-07-12 16:22:19 UTC

Patch posted:
http://www.redhat.com/archives/libguestfs/2011-July/msg00024.html

Comment 4 Richard W.M. Jones 2011-07-13 08:57:32 UTC

Fix pushed upstream and available in Rawhide in
libguestfs >= 1.11.15.