Denis Plotnikov
2019-Apr-15 11:06 UTC
[Libguestfs] [PATCH] daemon: drop error message check in do_part_expand_gpt
part-expand-gpt takes extreme cautions and doesn't proceed to writing
to the disk if the preliminary dry run of sgdisk has generated any
warnings on stdout.
This blocks the use of part-expand-gpt on disk shrink (with disk
resize being the main usecase for part-expand-gpt), because sgdisk dry
run produces a warning in that case.
So remove the excessive safety check, and leave it up to the caller.
Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
---
daemon/parted.c | 20 +-------------------
tests/gdisk/test-expand-gpt.pl | 24 +++++++++++++++++++++---
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/daemon/parted.c b/daemon/parted.c
index 070ed4790..2cc714d64 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -699,26 +699,8 @@ do_part_expand_gpt(const char *device)
{
CLEANUP_FREE char *err = NULL;
- /* If something is broken, sgdisk may try to correct it.
- * (e.g. recreate partition table and so on).
- * We do not want such behavior, so dry-run at first.*/
int r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR,
- "sgdisk", "--pretend", "-e",
device, NULL);
-
- if (r == -1) {
- reply_with_error ("%s --pretend -e %s: %s", "sgdisk",
device, err);
- return -1;
- }
- if (err && strlen(err) != 0) {
- /* Unexpected actions. */
- reply_with_error ("%s --pretend -e %s: %s", "sgdisk",
device, err);
- return -1;
- }
- free(err);
-
- /* Now we can do a real run. */
- r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR,
- "sgdisk", "-e", device, NULL);
+ "sgdisk", "-e", device, NULL);
if (r == -1) {
reply_with_error ("%s -e %s: %s", "sgdisk", device,
err);
diff --git a/tests/gdisk/test-expand-gpt.pl b/tests/gdisk/test-expand-gpt.pl
index 4d647f1af..f17d034ee 100755
--- a/tests/gdisk/test-expand-gpt.pl
+++ b/tests/gdisk/test-expand-gpt.pl
@@ -54,11 +54,29 @@ sub tests {
my $end_sectors = 100 * 1024 * 2 - $output;
die unless $end_sectors <= 34;
- # Negative tests.
+ # Negative test.
eval { $g->part_expand_gpt ("/dev/sdb") };
die unless $@;
- eval { $g->part_expand_gpt ("/dev/sda1") };
- die unless $@;
+
+ $g->close ();
+
+ # Disk shrink test
+ die if system ("qemu-img resize --shrink disk_gpt.img 50M
&>/dev/null");
+
+ $g = Sys::Guestfs->new ();
+
+ $g->add_drive ("disk_gpt.img", format => "qcow2");
+ $g->launch ();
+
+ die if $g->part_expand_gpt ("/dev/sda");
+
+ my $output = $g->debug ("sh", ["sgdisk",
"-p", "/dev/sda"]);
+ die if $output eq "";
+ $output =~ s/\n/ /g;
+ $output =~ s/.*last usable sector is (\d+).*/$1/g;
+
+ my $end_sectors = 50 * 1024 * 2 - $output;
+ die unless $end_sectors <= 34;
}
eval { tests() };
--
2.17.2
Richard W.M. Jones
2019-Apr-16 07:54 UTC
Re: [Libguestfs] [PATCH] daemon: drop error message check in do_part_expand_gpt
On Mon, Apr 15, 2019 at 02:06:29PM +0300, Denis Plotnikov wrote:> part-expand-gpt takes extreme cautions and doesn't proceed to writing > to the disk if the preliminary dry run of sgdisk has generated any > warnings on stdout. > > This blocks the use of part-expand-gpt on disk shrink (with disk > resize being the main usecase for part-expand-gpt), because sgdisk dry > run produces a warning in that case. > > So remove the excessive safety check, and leave it up to the caller. > > Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com> > Reviewed-by: Roman Kagan <rkagan@virtuozzo.com> > --- > daemon/parted.c | 20 +------------------- > tests/gdisk/test-expand-gpt.pl | 24 +++++++++++++++++++++--- > 2 files changed, 22 insertions(+), 22 deletions(-) > > diff --git a/daemon/parted.c b/daemon/parted.c > index 070ed4790..2cc714d64 100644 > --- a/daemon/parted.c > +++ b/daemon/parted.c > @@ -699,26 +699,8 @@ do_part_expand_gpt(const char *device) > { > CLEANUP_FREE char *err = NULL; > > - /* If something is broken, sgdisk may try to correct it. > - * (e.g. recreate partition table and so on). > - * We do not want such behavior, so dry-run at first.*/ > int r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR, > - "sgdisk", "--pretend", "-e", device, NULL); > - > - if (r == -1) { > - reply_with_error ("%s --pretend -e %s: %s", "sgdisk", device, err); > - return -1; > - } > - if (err && strlen(err) != 0) { > - /* Unexpected actions. */ > - reply_with_error ("%s --pretend -e %s: %s", "sgdisk", device, err); > - return -1; > - } > - free(err); > - > - /* Now we can do a real run. */ > - r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR, > - "sgdisk", "-e", device, NULL); > + "sgdisk", "-e", device, NULL); > > if (r == -1) { > reply_with_error ("%s -e %s: %s", "sgdisk", device, err); > diff --git a/tests/gdisk/test-expand-gpt.pl b/tests/gdisk/test-expand-gpt.pl > index 4d647f1af..f17d034ee 100755 > --- a/tests/gdisk/test-expand-gpt.pl > +++ b/tests/gdisk/test-expand-gpt.pl > @@ -54,11 +54,29 @@ sub tests { > my $end_sectors = 100 * 1024 * 2 - $output; > die unless $end_sectors <= 34; > > - # Negative tests. > + # Negative test. > eval { $g->part_expand_gpt ("/dev/sdb") }; > die unless $@; > - eval { $g->part_expand_gpt ("/dev/sda1") }; > - die unless $@; > + > + $g->close (); > + > + # Disk shrink test > + die if system ("qemu-img resize --shrink disk_gpt.img 50M &>/dev/null"); > + > + $g = Sys::Guestfs->new (); > + > + $g->add_drive ("disk_gpt.img", format => "qcow2"); > + $g->launch (); > + > + die if $g->part_expand_gpt ("/dev/sda"); > + > + my $output = $g->debug ("sh", ["sgdisk", "-p", "/dev/sda"]); > + die if $output eq ""; > + $output =~ s/\n/ /g; > + $output =~ s/.*last usable sector is (\d+).*/$1/g; > + > + my $end_sectors = 50 * 1024 * 2 - $output; > + die unless $end_sectors <= 34; > } > > eval { tests() };OK I'll run the tests on this one and if it works I'll push it. Thanks, 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/