9:43 a.m.
On Wed, Jan 14, 2015 at 07:48:26PM -0500, John Ferlan wrote:
On 01/14/2015 04:46 AM, Daniel P. Berrange wrote:
> On Tue, Jan 13, 2015 at 04:12:32PM -0500, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1138516
>>
>> The disk backend uses 'parted' in order to create and delete partitions
>> on the disk for use in the pool. At creation time, one can specify a
>> specific "name" for the volume as well as a specific volume target
>> format type. The name and volume target type "survive" only as long
>> as the pool is not refreshed or the libvirtd not restarted/reloaded.
>>
>> The action immediately prior to the calling all the backend refreshPool
>> API's is to clear out all the existing volumes from the pool. The
>> theory being the refresh will be able to find all elements of the
>> pool using some mechanism. The disk refreshPool backend will use the
>> libvirt_parthelper utility to read the partitions found on the disk
>> in order to regenerate the elements of the pool Unfortunately, since
>> the "name" and target format type cannot be encoded, the data is now
>> lost and the defaults are used (for the "name", the partition path
>> is used and the default of 'none' is used for the target format type).
>>
>> This patch solves this by adding the ability to save the XML generated
>> at create time into the stateDir and then use that during the refreshPool
>> backend API call to restore the specific fields that are lost.
>
> I don't really think we should be adding a lookaside cache for this.
Fair enough - although this may then turn into a documentation exercise.
I'm OK with that option - if that is what is desired.
> We should simply not permit arbitrary user supplied names for disk
> based volumes. The names should be required to match the defined
> naming scheme for disk partitions. I could have sworn that we had
> enforced that already when I first wrote this, but perhaps that
> check got lost somewhere along the way. Likewise for the format - we
> should just rely on partition table format data
I looked through some history and didn't see anywhere that supplied name
must match partition name was enforced, but there's a lot of code motion
and new features that may cloud the history. Perhaps it's the assumption
true in so many other pools that 'name' is what was used to create the
file, directory, lv, etc. There is code that fills in the name if not
provided with the partition name (which occurs after Refresh, reload,
restart).
Looking back some more, it seems I did not ever enforce it - I just
left it as something users had to set the "right way" by convention.
ie I just expected callers to always use the right "sdaNNN" value
I'm not quite sure how one could enforce a name given that
parted
creates the partition name. One would have to know the device name of
the pool and the numerical sequence that parted would use (easy perhaps
know that perhaps partitions 1 & 2 exist that 3 will be next). However,
there's not necessarily a guarantee of that is there?
For MS-DOS partition tables the partition entry number directly
corresponds to the device name suffix. eg partition 3 corresponds
to /dev/sda3 always and is happy to be sparse. eg if you create
3 partitions and then delete partition 2, you'll end up with
/dev/sda1 and /dev/sda3
Testing with parted the same seems to be true of GPT tables too.
Reading the libvirt docs - the implication is that the volume
"<name>"
must only be unique across the pool, so the following is "valid":
virsh vol-create-as disk-pool vol-linux --format linux 1G
So without assuming/describing the naming scheme of the underlying
parted how do we enforce that the name (vol-linux) is the same as what
parted generates? Or how do we enforce that the name provided ends up
being the same as the partition created? It's kind of a chicken/egg issue.
Well with disk partitions we know the device name
eg the pool knows we're operating on '/dev/sda', so when we create
it can we validate that the volume name is '/dev/sdaNNN' by kjust
doing a string prefix match
Not much is done with the target.format in the backend even though
it's
described as providing the "partition type". When not provided on the
command line like above it defaults to 'none'. Other than for dos label
pools that would need to have extended partition before creating a
logical partition, there's not much "use" for the field other than to
perhaps store "something" for "someone" (until refresh, reload,
restart
loses it).
When you create a partition you can specify filesystem type for that
partition, even though it is somewhat redundant as it is not directly
connected to the filesystem that you actually mkfs. eg this list of
types from fdisk
Command (m for help): l
0 Empty 24 NEC DOS 81 Minix / old Lin bf Solaris
1 FAT12 27 Hidden NTFS Win 82 Linux swap / So c1 DRDOS/sec (FAT-
2 XENIX root 39 Plan 9 83 Linux c4 DRDOS/sec (FAT-
3 XENIX usr 3c PartitionMagic 84 OS/2 hidden C: c6 DRDOS/sec (FAT-
4 FAT16 <32M 40 Venix 80286 85 Linux extended c7 Syrinx
5 Extended 41 PPC PReP Boot 86 NTFS volume set da Non-FS data
6 FAT16 42 SFS 87 NTFS volume set db CP/M / CTOS / .
7 HPFS/NTFS/exFAT 4d QNX4.x 88 Linux plaintext de Dell Utility
8 AIX 4e QNX4.x 2nd part 8e Linux LVM df BootIt
9 AIX bootable 4f QNX4.x 3rd part 93 Amoeba e1 DOS access
a OS/2 Boot Manag 50 OnTrack DM 94 Amoeba BBT e3 DOS R/O
b W95 FAT32 51 OnTrack DM6 Aux 9f BSD/OS e4 SpeedStor
c W95 FAT32 (LBA) 52 CP/M a0 IBM Thinkpad hi eb BeOS fs
e W95 FAT16 (LBA) 53 OnTrack DM6 Aux a5 FreeBSD ee GPT
f W95 Ext'd (LBA) 54 OnTrackDM6 a6 OpenBSD ef EFI (FAT-12/16/
10 OPUS 55 EZ-Drive a7 NeXTSTEP f0 Linux/PA-RISC b
11 Hidden FAT12 56 Golden Bow a8 Darwin UFS f1 SpeedStor
12 Compaq diagnost 5c Priam Edisk a9 NetBSD f4 SpeedStor
14 Hidden FAT16 <3 61 SpeedStor ab Darwin boot f2 DOS secondary
16 Hidden FAT16 63 GNU HURD or Sys af HFS / HFS+ fb VMware VMFS
17 Hidden HPFS/NTF 64 Novell Netware b7 BSDI fs fc VMware VMKCORE
18 AST SmartSleep 65 Novell Netware b8 BSDI swap fd Linux raid auto
1b Hidden W95 FAT3 70 DiskSecure Mult bb Boot Wizard hid fe LANstep
1c Hidden W95 FAT3 75 PC/IX be Solaris boot ff BBT
1e Hidden W95 FAT1 80 Old Minix
I did make a start on defining this for the XML:
/*
* XXX: these are basically partition types.
*
* fdisk has a bazillion partition ID types parted has
* practically none, and splits the * info across 3
* different attributes.
*
* So this is a semi-generic set
*/
typedef enum {
VIR_STORAGE_VOL_DISK_NONE = 0,
VIR_STORAGE_VOL_DISK_LINUX,
VIR_STORAGE_VOL_DISK_FAT16,
VIR_STORAGE_VOL_DISK_FAT32,
VIR_STORAGE_VOL_DISK_LINUX_SWAP,
VIR_STORAGE_VOL_DISK_LINUX_LVM,
VIR_STORAGE_VOL_DISK_LINUX_RAID,
VIR_STORAGE_VOL_DISK_EXTENDED,
VIR_STORAGE_VOL_DISK_LAST,
} virStorageVolFormatDisk;
VIR_ENUM_DECL(virStorageVolFormatDisk)
but then storage_backend_disk never made use of this information
I'm not sure how important this is, but if we do want to make use
of 'format' for the disk pool, this partition types are what I'd
map it too.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|