9:01 a.m.
Hi,
attached there are few patches to implement a way to get the label of
GPT partitions (refactoring an existing function and adding a new
daemon API) and using it in virt-resize to restore them when copying
partitions.
Thanks,
Pino Toscano (3):
daemon: parted: refactor sgdisk info parsing code
New API: part-get-name (RHBZ#593511).
resize: preserve GPT partition names (RHBZ#1060404).
daemon/parted.c | 86 ++++++++++++++++++++++++++++++++++++++++++----------
generator/actions.ml | 13 ++++++++
resize/resize.ml | 19 +++++++++++-
src/MAX_PROC_NR | 2 +-
4 files changed, 102 insertions(+), 18 deletions(-)
--
1.8.3.1
9:01 a.m.
New subject: [PATCH 1/3] daemon: parted: refactor sgdisk info parsing code
Isolate in an own function the code that runs sgdisk and parse a field
of it (using an extraction function passed as parameter), using it for
the GUID type.
This is just code motion, no actual behaviour changes.
---
daemon/parted.c | 53 +++++++++++++++++++++++++++++++++++++----------------
1 file changed, 37 insertions(+), 16 deletions(-)
diff --git a/daemon/parted.c b/daemon/parted.c
index bd81986..5282adb 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -784,8 +784,9 @@ do_part_set_gpt_type(const char *device, int partnum, const char
*guid)
return 0;
}
-char *
-do_part_get_gpt_type(const char *device, int partnum)
+static char *
+sgdisk_info_extract_field (const char *device, int partnum, const char *field,
+ char *(*extract) (const char *path))
{
if (partnum <= 0) {
reply_with_error ("partition number must be >= 1");
@@ -814,6 +815,8 @@ do_part_get_gpt_type(const char *device, int partnum)
return NULL;
}
+ int fieldlen = strlen (field);
+
/* Parse the output of sgdisk -i:
* Partition GUID code: 21686148-6449-6E6F-744E-656564454649 (BIOS boot partition)
* Partition unique GUID: 19AEC5FE-D63A-4A15-9D37-6FCBFB873DC0
@@ -832,28 +835,22 @@ do_part_get_gpt_type(const char *device, int partnum)
/* Split the line in 2 at the colon */
char *colon = strchr (line, ':');
if (colon) {
-#define SEARCH "Partition GUID code"
- if (colon - line == strlen(SEARCH) &&
- memcmp (line, SEARCH, strlen(SEARCH)) == 0)
+ if (colon - line == fieldlen &&
+ memcmp (line, field, fieldlen) == 0)
{
-#undef SEARCH
/* The value starts after the colon */
char *value = colon + 1;
/* Skip any leading whitespace */
value += strspn (value, " \t");
- /* The value contains only valid GUID characters */
- size_t value_len = strspn (value, "-0123456789ABCDEF");
-
- char *ret = malloc (value_len + 1);
+ /* Extract the actual information from the field. */
+ char *ret = extract (value);
if (ret == NULL) {
- reply_with_perror ("malloc");
+ /* The extraction function already sends the error. */
return NULL;
}
- memcpy (ret, value, value_len);
- ret[value_len] = '\0';
return ret;
}
} else {
@@ -866,8 +863,32 @@ do_part_get_gpt_type(const char *device, int partnum)
}
}
- /* If we got here it means we didn't find the Partition GUID code */
- reply_with_error ("sgdisk output did not contain Partition GUID code. "
- "See LIBGUESTFS_DEBUG output for more details");
+ /* If we got here it means we didn't find the field */
+ reply_with_error ("sgdisk output did not contain '%s'. "
+ "See LIBGUESTFS_DEBUG output for more details", field);
return NULL;
}
+
+static char *
+extract_uuid (const char *value)
+{
+ /* The value contains only valid GUID characters */
+ size_t value_len = strspn (value, "-0123456789ABCDEF");
+
+ char *ret = malloc (value_len + 1);
+ if (ret == NULL) {
+ reply_with_perror ("malloc");
+ return NULL;
+ }
+
+ memcpy (ret, value, value_len);
+ ret[value_len] = '\0';
+ return ret;
+}
+
+char *
+do_part_get_gpt_type (const char *device, int partnum)
+{
+ return sgdisk_info_extract_field (device, partnum,
+ "Partition GUID code", extract_uuid);
+}
--
1.8.3.1
9:56 a.m.
New subject: [PATCH 1/3] daemon: parted: refactor sgdisk info parsing code
On Tue, Feb 04, 2014 at 04:01:31PM +0100, Pino Toscano wrote:
@@ -832,28 +835,22 @@ do_part_get_gpt_type(const char *device, int
partnum)
/* Split the line in 2 at the colon */
char *colon = strchr (line, ':');
if (colon) {
-#define SEARCH "Partition GUID code"
- if (colon - line == strlen(SEARCH) &&
- memcmp (line, SEARCH, strlen(SEARCH)) == 0)
+ if (colon - line == fieldlen &&
+ memcmp (line, field, fieldlen) == 0)
Maybe use STRPREFIX macro here?
Looks fine, ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
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/
10:38 a.m.
New subject: [PATCH 1/3] daemon: parted: refactor sgdisk info parsing code
On Tuesday 04 February 2014 15:56:54 Richard W.M. Jones wrote:
On Tue, Feb 04, 2014 at 04:01:31PM +0100, Pino Toscano wrote:
> @@ -832,28 +835,22 @@ do_part_get_gpt_type(const char *device, int
> partnum)>
> /* Split the line in 2 at the colon */
> char *colon = strchr (line, ':');
> if (colon) {
>
> -#define SEARCH "Partition GUID code"
> - if (colon - line == strlen(SEARCH) &&
> - memcmp (line, SEARCH, strlen(SEARCH)) == 0)
> + if (colon - line == fieldlen &&
> + memcmp (line, field, fieldlen) == 0)
Maybe use STRPREFIX macro here?
STRPREFIX calls strlen every time it is used, and uses strncmp. While
not a big deal, the code above is used in a loop, so doing strlen once
and comparing bits using memcmp should be faster.
Yes, I know it is not performance-critical code, but since it was
already there and working fine, IMHO could continue to do so :)
--
Pino Toscano
9:01 a.m.
New subject: [PATCH 2/3] New API: part-get-name (RHBZ#593511).
Counterpart of part-set-name, it uses sgdisk (hence needs the "gdisk"
feature) to query for the label/name of a partition in a GPT layout.
---
daemon/parted.c | 33 +++++++++++++++++++++++++++++++++
generator/actions.ml | 13 +++++++++++++
src/MAX_PROC_NR | 2 +-
3 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/daemon/parted.c b/daemon/parted.c
index 5282adb..83f2411 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -886,9 +886,42 @@ extract_uuid (const char *value)
return ret;
}
+static char *
+extract_optionally_quoted (const char *value)
+{
+ size_t value_len = strlen (value);
+
+ if (value_len >= 2 &&
+ ((value[0] == '\'' && value[value_len - 1] == '\'')
||
+ (value[0] == '"' && value[value_len - 1] ==
'"'))) {
+ value_len -= 2;
+ ++value;
+ }
+
+ char *ret = strndup (value, value_len);
+ if (ret == NULL) {
+ reply_with_perror ("strndup");
+ return NULL;
+ }
+
+ return ret;
+}
+
char *
do_part_get_gpt_type (const char *device, int partnum)
{
return sgdisk_info_extract_field (device, partnum,
"Partition GUID code", extract_uuid);
}
+
+char *
+do_part_get_name (const char *device, int partnum)
+{
+ char *parttype = do_part_get_parttype (device);
+ if (STREQ (parttype, "gpt"))
+ return sgdisk_info_extract_field (device, partnum,
+ "Partition name",
extract_optionally_quoted);
+
+ reply_with_error ("cannot get the partition name from '%s' layouts",
parttype);
+ return NULL;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index 176de98..b665149 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -11766,6 +11766,19 @@ enables all the other flags, if they are not specified already.
=back" };
+ { defaults with
+ name = "part_get_name";
+ style = RString "name", [Device "device"; Int
"partnum"], [];
+ proc_nr = Some 416;
+ optional = Some "gdisk";
+ shortdesc = "get partition name";
+ longdesc = "\
+This gets the partition name on partition numbered C<partnum> on
+device C<device>. Note that partitions are numbered from 1.
+
+The partition name can only be read on certain types of partition
+table. This works on C<gpt> but not on C<mbr> partitions." };
+
]
(* Non-API meta-commands available only in guestfish.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 21c8d99..1c105f1 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-415
+416
--
1.8.3.1
9:59 a.m.
New subject: [PATCH 2/3] New API: part-get-name (RHBZ#593511).
On Tue, Feb 04, 2014 at 04:01:32PM +0100, Pino Toscano wrote:
+static char *
+extract_optionally_quoted (const char *value)
+{
+ size_t value_len = strlen (value);
+
+ if (value_len >= 2 &&
+ ((value[0] == '\'' && value[value_len - 1] ==
'\'') ||
+ (value[0] == '"' && value[value_len - 1] ==
'"'))) {
+ value_len -= 2;
+ ++value;
+ }
+
+ char *ret = strndup (value, value_len);
+ if (ret == NULL) {
+ reply_with_perror ("strndup");
+ return NULL;
+ }
+
+ return ret;
+}
My spidey sense asks what happens if the value contains quote
characters? I wonder if sgdisk escapes them.
Rest of the patch looks fine, but I think you should try setting the
partition name to a few weird values and see what sgdisk does.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
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/
10:20 a.m.
New subject: [PATCH 2/3] New API: part-get-name (RHBZ#593511).
On Tuesday 04 February 2014 15:59:36 Richard W.M. Jones wrote:
On Tue, Feb 04, 2014 at 04:01:32PM +0100, Pino Toscano wrote:
> +static char *
> +extract_optionally_quoted (const char *value)
> +{
> + size_t value_len = strlen (value);
> +
> + if (value_len >= 2 &&
> + ((value[0] == '\'' && value[value_len - 1] ==
'\'') ||
> + (value[0] == '"' && value[value_len - 1] ==
'"'))) {
> + value_len -= 2;
> + ++value;
> + }
> +
> + char *ret = strndup (value, value_len);
> + if (ret == NULL) {
> + reply_with_perror ("strndup");
> + return NULL;
> + }
> +
> + return ret;
> +}
My spidey sense asks what happens if the value contains quote
characters? I wonder if sgdisk escapes them.
Rest of the patch looks fine, but I think you should try setting the
partition name to a few weird values and see what sgdisk does.
It seems sgdisk just picks whatever it is being passed to it:
<rescue> sgdisk -c 3:test /dev/sda
[ 129.149627] sda:
sda1 sda2 sda3
The operation has completed successfully.
<rescue> sgdisk -i 3 /dev/sda | grep name
Partition
name: 'test'
<rescue> sgdisk -c 3:test /dev/sda
[ 146.992530] sda:
sda1 sda2 sda3
The operation has completed successfully.
<rescue> sgdisk -i 3 /dev/sda | grep name
Partition
name: 'test'
<rescue> sgdisk -c 3:"test" /dev/sda
[
194.584171] sda: sda1 sda2 sda3
The operation has completed successfully.
<rescue> sgdisk -i 3 /dev/sda | grep name
Partition
name: 'test'
<rescue> sgdisk -c 3:'test' /dev/sda
[
202.042122] sda: sda1 sda2 sda3
The operation has completed successfully.
<rescue> sgdisk -i 3 /dev/sda | grep name
Partition
name: 'test'
<rescue> sgdisk -c 3:"'test'" /dev/sda
[ 211.002185] sda: sda1 sda2 sda3
The operation has completed successfully.
<rescue> sgdisk -i 3 /dev/sda | grep name
Partition
name: ''test''
<rescue> sgdisk -c 3:'"test"' /dev/sda
[ 217.441540] sda: sda1 sda2 sda3
The operation has completed successfully.
<rescue> sgdisk -i 3 /dev/sda | grep name
Partition
name: '"test"'
So the output is always single-quoted, with other quote characters just
appearing as they were in the partition label.
--
Pino Toscano
10:30 a.m.
New subject: [PATCH 2/3] New API: part-get-name (RHBZ#593511).
On Tue, Feb 04, 2014 at 05:20:22PM +0100, Pino Toscano wrote:
On Tuesday 04 February 2014 15:59:36 Richard W.M. Jones wrote:
> On Tue, Feb 04, 2014 at 04:01:32PM +0100, Pino Toscano wrote:
> > +static char *
> > +extract_optionally_quoted (const char *value)
> > +{
> > + size_t value_len = strlen (value);
> > +
> > + if (value_len >= 2 &&
> > + ((value[0] == '\'' && value[value_len - 1] ==
'\'') ||
> > + (value[0] == '"' && value[value_len - 1] ==
'"'))) {
> > + value_len -= 2;
> > + ++value;
> > + }
> > +
> > + char *ret = strndup (value, value_len);
> > + if (ret == NULL) {
> > + reply_with_perror ("strndup");
> > + return NULL;
> > + }
> > +
> > + return ret;
> > +}
>
> My spidey sense asks what happens if the value contains quote
> characters? I wonder if sgdisk escapes them.
>
> Rest of the patch looks fine, but I think you should try setting the
> partition name to a few weird values and see what sgdisk does.
It seems sgdisk just picks whatever it is being passed to it:
><rescue> sgdisk -c 3:test /dev/sda
[ 129.149627] sda: sda1 sda2 sda3
The operation has completed successfully.
><rescue> sgdisk -i 3 /dev/sda | grep name
Partition name: 'test'
><rescue> sgdisk -c 3:test /dev/sda
[ 146.992530] sda: sda1 sda2 sda3
The operation has completed successfully.
><rescue> sgdisk -i 3 /dev/sda | grep name
Partition name: 'test'
><rescue> sgdisk -c 3:"test" /dev/sda
[ 194.584171] sda: sda1 sda2 sda3
The operation has completed successfully.
><rescue> sgdisk -i 3 /dev/sda | grep name
Partition name: 'test'
><rescue> sgdisk -c 3:'test' /dev/sda
[ 202.042122] sda: sda1 sda2 sda3
The operation has completed successfully.
><rescue> sgdisk -i 3 /dev/sda | grep name
Partition name: 'test'
><rescue> sgdisk -c 3:"'test'" /dev/sda
[ 211.002185] sda: sda1 sda2 sda3
The operation has completed successfully.
><rescue> sgdisk -i 3 /dev/sda | grep name
Partition name: ''test''
><rescue> sgdisk -c 3:'"test"' /dev/sda
[ 217.441540] sda: sda1 sda2 sda3
The operation has completed successfully.
><rescue> sgdisk -i 3 /dev/sda | grep name
Partition name: '"test"'
So the output is always single-quoted, with other quote characters just
appearing as they were in the partition label.
OK, I'm sure that will catch us out in future when sgdisk
changes/fixes it :-(
ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
9:01 a.m.
New subject: [PATCH 3/3] resize: preserve GPT partition names (RHBZ#1060404).
Save the partition names/labels of the source partitions, and restore
them after the partition copy.
---
resize/resize.ml | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/resize/resize.ml b/resize/resize.ml
index 191be83..c1794ed 100644
--- a/resize/resize.ml
+++ b/resize/resize.ml
@@ -49,6 +49,7 @@ type partition = {
p_bootable : bool; (* Is it bootable? *)
p_id : partition_id; (* Partition (MBR/GPT) ID. *)
p_type : partition_content; (* Content type and content size. *)
+ p_label : string option; (* Label/name. *)
(* What we're going to do: *)
mutable p_operation : partition_operation;
@@ -84,7 +85,12 @@ let rec debug_partition p =
| MBR_ID i -> sprintf "0x%x" i
| GPT_Type i -> i
);
- eprintf "\tcontent: %s\n" (string_of_partition_content p.p_type)
+ eprintf "\tcontent: %s\n" (string_of_partition_content p.p_type);
+ eprintf "\tlabel: %s\n"
+ (match p.p_label with
+ | Some label -> label
+ | None -> "(none)"
+ )
and string_of_partition_content = function
| ContentUnknown -> "unknown data"
| ContentPV sz -> sprintf "LVM PV (%Ld bytes)" sz
@@ -459,9 +465,13 @@ read the man page virt-resize(1).
let typ =
if is_extended_partition id then ContentExtendedPartition
else get_partition_content name in
+ let label =
+ try Some (g#part_get_name "/dev/sda" part_num)
+ with G.Error _ -> None in
{ p_name = name; p_part = part;
p_bootable = bootable; p_id = id; p_type = typ;
+ p_label = label;
p_operation = OpCopy; p_target_partnum = 0;
p_target_start = 0L; p_target_end = 0L }
) parts in
@@ -1040,6 +1050,7 @@ read the man page virt-resize(1).
p_part = { G.part_num = 0l; part_start = 0L; part_end = 0L;
part_size = 0L };
p_bootable = false; p_id = No_ID; p_type = ContentUnknown;
+ p_label = None;
(* Target information is meaningful. *)
p_operation = OpIgnore;
@@ -1117,6 +1128,12 @@ read the man page virt-resize(1).
if p.p_bootable then
g#part_set_bootable "/dev/sdb" p.p_target_partnum true;
+ (match p.p_label with
+ | Some label ->
+ g#part_set_name "/dev/sdb" p.p_target_partnum label;
+ | None -> ()
+ );
+
match parttype, p.p_id with
| GPT, GPT_Type gpt_type ->
g#part_set_gpt_type "/dev/sdb" p.p_target_partnum gpt_type
--
1.8.3.1
10 a.m.
New subject: [PATCH 3/3] resize: preserve GPT partition names (RHBZ#1060404).
On Tue, Feb 04, 2014 at 04:01:33PM +0100, Pino Toscano wrote:
Save the partition names/labels of the source partitions, and
restore
them after the partition copy.
---
resize/resize.ml | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/resize/resize.ml b/resize/resize.ml
index 191be83..c1794ed 100644
--- a/resize/resize.ml
+++ b/resize/resize.ml
@@ -49,6 +49,7 @@ type partition = {
p_bootable : bool; (* Is it bootable? *)
p_id : partition_id; (* Partition (MBR/GPT) ID. *)
p_type : partition_content; (* Content type and content size. *)
+ p_label : string option; (* Label/name. *)
(* What we're going to do: *)
mutable p_operation : partition_operation;
@@ -84,7 +85,12 @@ let rec debug_partition p =
| MBR_ID i -> sprintf "0x%x" i
| GPT_Type i -> i
);
- eprintf "\tcontent: %s\n" (string_of_partition_content p.p_type)
+ eprintf "\tcontent: %s\n" (string_of_partition_content p.p_type);
+ eprintf "\tlabel: %s\n"
+ (match p.p_label with
+ | Some label -> label
+ | None -> "(none)"
+ )
and string_of_partition_content = function
| ContentUnknown -> "unknown data"
| ContentPV sz -> sprintf "LVM PV (%Ld bytes)" sz
@@ -459,9 +465,13 @@ read the man page virt-resize(1).
let typ =
if is_extended_partition id then ContentExtendedPartition
else get_partition_content name in
+ let label =
+ try Some (g#part_get_name "/dev/sda" part_num)
+ with G.Error _ -> None in
{ p_name = name; p_part = part;
p_bootable = bootable; p_id = id; p_type = typ;
+ p_label = label;
p_operation = OpCopy; p_target_partnum = 0;
p_target_start = 0L; p_target_end = 0L }
) parts in
@@ -1040,6 +1050,7 @@ read the man page virt-resize(1).
p_part = { G.part_num = 0l; part_start = 0L; part_end = 0L;
part_size = 0L };
p_bootable = false; p_id = No_ID; p_type = ContentUnknown;
+ p_label = None;
(* Target information is meaningful. *)
p_operation = OpIgnore;
@@ -1117,6 +1128,12 @@ read the man page virt-resize(1).
if p.p_bootable then
g#part_set_bootable "/dev/sdb" p.p_target_partnum true;
+ (match p.p_label with
+ | Some label ->
+ g#part_set_name "/dev/sdb" p.p_target_partnum label;
+ | None -> ()
+ );
+
match parttype, p.p_id with
| GPT, GPT_Type gpt_type ->
g#part_set_gpt_type "/dev/sdb" p.p_target_partnum gpt_type
--
1.8.3.1
ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
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
3977
days inactive
3977
days old
9 comments
2 participants
participants (2)
-
Pino Toscano -
Richard W.M. Jones