Richard W.M. Jones
2016-Jun-02 15:11 UTC
[Libguestfs] [PATCH 0/3] builder: Warn if --output is a host partition.
Rather complex patch to solve a small user error. Warn if the user is doing something like: virt-builder -o /dev/sdX1 Rich.
Richard W.M. Jones
2016-Jun-02 15:11 UTC
[Libguestfs] [PATCH 1/3] mllib: Add bindings for makedev(3), major(3) and minor(3).
---
mllib/Makefile.am | 3 +++
mllib/dev_t-c.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
mllib/dev_t.ml | 21 +++++++++++++++++++++
mllib/dev_t.mli | 28 ++++++++++++++++++++++++++++
4 files changed, 104 insertions(+)
create mode 100644 mllib/dev_t-c.c
create mode 100644 mllib/dev_t.ml
create mode 100644 mllib/dev_t.mli
diff --git a/mllib/Makefile.am b/mllib/Makefile.am
index 25f0081..5e30755 100644
--- a/mllib/Makefile.am
+++ b/mllib/Makefile.am
@@ -28,6 +28,7 @@ CLEANFILES = *~ *.annot *.cmi *.cmo *.cmx *.cmxa *.o
SOURCES_MLI = \
common_utils.mli \
+ dev_t.mli \
fsync.mli \
JSON.mli \
mkdtemp.mli \
@@ -40,6 +41,7 @@ SOURCES_ML = \
guestfs_config.ml \
libdir.ml \
common_gettext.ml \
+ dev_t.ml \
common_utils.ml \
fsync.ml \
progress.ml \
@@ -52,6 +54,7 @@ SOURCES_ML = \
SOURCES_C = \
../fish/progress.c \
../fish/uri.c \
+ dev_t-c.c \
fsync-c.c \
mkdtemp-c.c \
progress-c.c \
diff --git a/mllib/dev_t-c.c b/mllib/dev_t-c.c
new file mode 100644
index 0000000..036b60d
--- /dev/null
+++ b/mllib/dev_t-c.c
@@ -0,0 +1,52 @@
+/* libguestfs OCaml tools common code
+ * Copyright (C) 2016 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/types.h>
+
+#include <caml/mlvalues.h>
+
+/* OCaml doesn't bind the dev_t calls makedev, major and minor. */
+
+extern value guestfs_int_mllib_dev_t_makedev (value majv, value minv);
+extern value guestfs_int_mllib_dev_t_major (value devv);
+extern value guestfs_int_mllib_dev_t_minor (value devv);
+
+/* NB: This is a "noalloc" call. */
+value
+guestfs_int_mllib_dev_t_makedev (value majv, value minv)
+{
+ return Val_int (makedev (Int_val (majv), Int_val (minv)));
+}
+
+/* NB: This is a "noalloc" call. */
+value
+guestfs_int_mllib_dev_t_major (value devv)
+{
+ return Val_int (major (Int_val (devv)));
+}
+
+/* NB: This is a "noalloc" call. */
+value
+guestfs_int_mllib_dev_t_minor (value devv)
+{
+ return Val_int (minor (Int_val (devv)));
+}
diff --git a/mllib/dev_t.ml b/mllib/dev_t.ml
new file mode 100644
index 0000000..143954a
--- /dev/null
+++ b/mllib/dev_t.ml
@@ -0,0 +1,21 @@
+(* libguestfs OCaml tools common code
+ * Copyright (C) 2016 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+external makedev : int -> int -> int =
"guestfs_int_mllib_dev_t_makedev" "noalloc"
+external major : int -> int = "guestfs_int_mllib_dev_t_major"
"noalloc"
+external minor : int -> int = "guestfs_int_mllib_dev_t_minor"
"noalloc"
diff --git a/mllib/dev_t.mli b/mllib/dev_t.mli
new file mode 100644
index 0000000..340ead0
--- /dev/null
+++ b/mllib/dev_t.mli
@@ -0,0 +1,28 @@
+(* virt-resize
+ * Copyright (C) 2016 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+(** Bindings for [dev_t] related functions [makedev], [major] and [minor]. *)
+
+val makedev : int -> int -> int
+(** makedev(3) *)
+
+val major : int -> int
+(** major(3) *)
+
+val minor : int -> int
+(** minor(3) *)
--
2.7.4
Richard W.M. Jones
2016-Jun-02 15:11 UTC
[Libguestfs] [PATCH 2/3] mllib: Add Common_utils.is_partition function.
Returns true if the device is a host partition. The only tedious bit of this patch is that now Common_utils depends on Dev_t so we have to add extra objects every time something links with Common_utils. --- builder/Makefile.am | 2 ++ customize/Makefile.am | 2 ++ dib/Makefile.am | 2 ++ get-kernel/Makefile.am | 2 ++ mllib/common_utils.ml | 13 +++++++++++++ mllib/common_utils.mli | 4 ++++ resize/Makefile.am | 2 ++ sparsify/Makefile.am | 2 ++ sysprep/Makefile.am | 2 ++ v2v/Makefile.am | 4 ++++ 10 files changed, 35 insertions(+) diff --git a/builder/Makefile.am b/builder/Makefile.am index 7f363ab..974e80c 100644 --- a/builder/Makefile.am +++ b/builder/Makefile.am @@ -89,6 +89,7 @@ SOURCES_ML = \ builder.ml SOURCES_C = \ + ../mllib/dev_t-c.c \ ../mllib/fsync-c.c \ ../mllib/uri-c.c \ ../mllib/mkdtemp-c.c \ @@ -134,6 +135,7 @@ BOBJECTS = \ $(top_builddir)/mllib/libdir.cmo \ $(top_builddir)/mllib/guestfs_config.cmo \ $(top_builddir)/mllib/common_gettext.cmo \ + $(top_builddir)/mllib/dev_t.cmo \ $(top_builddir)/mllib/common_utils.cmo \ $(top_builddir)/mllib/fsync.cmo \ $(top_builddir)/mllib/planner.cmo \ diff --git a/customize/Makefile.am b/customize/Makefile.am index e921c5c..34b40d5 100644 --- a/customize/Makefile.am +++ b/customize/Makefile.am @@ -69,6 +69,7 @@ SOURCES_C = \ ../fish/uri.c \ ../fish/file-edit.c \ ../fish/file-edit.h \ + ../mllib/dev_t-c.c \ ../mllib/uri-c.c \ crypt-c.c \ perl_edit-c.c @@ -93,6 +94,7 @@ virt_customize_CFLAGS = \ BOBJECTS = \ $(top_builddir)/mllib/guestfs_config.cmo \ $(top_builddir)/mllib/common_gettext.cmo \ + $(top_builddir)/mllib/dev_t.cmo \ $(top_builddir)/mllib/common_utils.cmo \ $(top_builddir)/mllib/regedit.cmo \ $(top_builddir)/mllib/URI.cmo \ diff --git a/dib/Makefile.am b/dib/Makefile.am index 5a3a7c7..d3917db 100644 --- a/dib/Makefile.am +++ b/dib/Makefile.am @@ -33,6 +33,7 @@ SOURCES_ML = \ dib.ml SOURCES_C = \ + ../mllib/dev_t-c.c \ ../mllib/mkdtemp-c.c bin_PROGRAMS @@ -57,6 +58,7 @@ BOBJECTS = \ $(top_builddir)/mllib/libdir.cmo \ $(top_builddir)/mllib/guestfs_config.cmo \ $(top_builddir)/mllib/common_gettext.cmo \ + $(top_builddir)/mllib/dev_t.cmo \ $(top_builddir)/mllib/common_utils.cmo \ $(top_builddir)/mllib/mkdtemp.cmo \ $(SOURCES_ML:.ml=.cmo) diff --git a/get-kernel/Makefile.am b/get-kernel/Makefile.am index dd13e63..b6e924f 100644 --- a/get-kernel/Makefile.am +++ b/get-kernel/Makefile.am @@ -27,6 +27,7 @@ SOURCES_ML = \ get_kernel.ml SOURCES_C = \ + ../mllib/dev_t-c.c \ ../mllib/uri-c.c \ ../fish/uri.c @@ -56,6 +57,7 @@ BOBJECTS = \ $(top_builddir)/mllib/libdir.cmo \ $(top_builddir)/mllib/guestfs_config.cmo \ $(top_builddir)/mllib/common_gettext.cmo \ + $(top_builddir)/mllib/dev_t.cmo \ $(top_builddir)/mllib/common_utils.cmo \ $(top_builddir)/mllib/URI.cmo \ $(SOURCES_ML:.ml=.cmo) diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml index 983a2e6..dfffae3 100644 --- a/mllib/common_utils.ml +++ b/mllib/common_utils.ml @@ -817,6 +817,19 @@ let is_char_device file try (Unix.stat file).Unix.st_kind = Unix.S_CHR with Unix.Unix_error _ -> false +let is_partition dev + try + if not (is_block_device dev) then false + else ( + let rdev = (Unix.stat dev).Unix.st_rdev in + let major = Dev_t.major rdev in + let minor = Dev_t.minor rdev in + let path = sprintf "/sys/dev/block/%d:%d/partition" major minor in + Unix.access path [F_OK]; + true + ) + with Unix.Unix_error _ -> false + (* Annoyingly Sys.is_directory throws an exception on failure * (RHBZ#1022431). *) diff --git a/mllib/common_utils.mli b/mllib/common_utils.mli index 5475b3e..3fcb602 100644 --- a/mllib/common_utils.mli +++ b/mllib/common_utils.mli @@ -293,6 +293,10 @@ val is_char_device : string -> bool val is_directory : string -> bool (** These don't throw exceptions, unlike the [Sys] functions. *) +val is_partition : string -> bool +(** Return true if the host device [dev] is a partition. If it's + anything else, or missing, returns false. *) + val absolute_path : string -> string (** Convert any path to an absolute path. *) diff --git a/resize/Makefile.am b/resize/Makefile.am index 06e5839..929f61c 100644 --- a/resize/Makefile.am +++ b/resize/Makefile.am @@ -30,6 +30,7 @@ SOURCES_ML = \ resize.ml SOURCES_C = \ + ../mllib/dev_t-c.c \ ../mllib/fsync-c.c \ ../fish/progress.c \ ../mllib/progress-c.c \ @@ -58,6 +59,7 @@ BOBJECTS = \ $(top_builddir)/mllib/URI.cmo \ $(top_builddir)/mllib/guestfs_config.cmo \ $(top_builddir)/mllib/common_gettext.cmo \ + $(top_builddir)/mllib/dev_t.cmo \ $(top_builddir)/mllib/common_utils.cmo \ $(SOURCES_ML:.ml=.cmo) XOBJECTS = $(BOBJECTS:.cmo=.cmx) diff --git a/sparsify/Makefile.am b/sparsify/Makefile.am index 68f7fe7..7fd27cb 100644 --- a/sparsify/Makefile.am +++ b/sparsify/Makefile.am @@ -37,6 +37,7 @@ SOURCES_ML = \ SOURCES_C = \ ../fish/progress.c \ + ../mllib/dev_t-c.c \ ../mllib/progress-c.c \ statvfs-c.c @@ -57,6 +58,7 @@ virt_sparsify_CFLAGS = \ BOBJECTS = \ $(top_builddir)/mllib/guestfs_config.cmo \ $(top_builddir)/mllib/common_gettext.cmo \ + $(top_builddir)/mllib/dev_t.cmo \ $(top_builddir)/mllib/common_utils.cmo \ $(top_builddir)/mllib/progress.cmo \ $(SOURCES_ML:.ml=.cmo) diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am index 7913484..3783a0b 100644 --- a/sysprep/Makefile.am +++ b/sysprep/Makefile.am @@ -80,6 +80,7 @@ SOURCES_ML = \ main.ml SOURCES_C = \ + ../mllib/dev_t-c.c \ ../mllib/uri-c.c \ ../mllib/mkdtemp-c.c \ ../customize/crypt-c.c \ @@ -106,6 +107,7 @@ virt_sysprep_CFLAGS = \ BOBJECTS = \ $(top_builddir)/mllib/guestfs_config.cmo \ $(top_builddir)/mllib/common_gettext.cmo \ + $(top_builddir)/mllib/dev_t.cmo \ $(top_builddir)/mllib/common_utils.cmo \ $(top_builddir)/mllib/URI.cmo \ $(top_builddir)/mllib/mkdtemp.cmo \ diff --git a/v2v/Makefile.am b/v2v/Makefile.am index f4bed83..e1f016b 100644 --- a/v2v/Makefile.am +++ b/v2v/Makefile.am @@ -114,6 +114,7 @@ SOURCES_ML = \ v2v.ml SOURCES_C = \ + ../mllib/dev_t-c.c \ ../mllib/mkdtemp-c.c \ domainxml-c.c \ changeuid-c.c \ @@ -138,6 +139,7 @@ virt_v2v_CFLAGS = \ BOBJECTS = \ $(top_builddir)/mllib/guestfs_config.cmo \ $(top_builddir)/mllib/common_gettext.cmo \ + $(top_builddir)/mllib/dev_t.cmo \ $(top_builddir)/mllib/common_utils.cmo \ $(top_builddir)/mllib/regedit.cmo \ $(top_builddir)/mllib/mkdtemp.cmo \ @@ -187,6 +189,7 @@ virt_v2v_LINK = \ $(OBJECTS) -o $@ virt_v2v_copy_to_local_SOURCES = \ + ../mllib/dev_t-c.c \ domainxml-c.c \ utils-c.c \ xml-c.c @@ -203,6 +206,7 @@ virt_v2v_copy_to_local_CFLAGS = \ COPY_TO_LOCAL_BOBJECTS = \ $(top_builddir)/mllib/guestfs_config.cmo \ $(top_builddir)/mllib/common_gettext.cmo \ + $(top_builddir)/mllib/dev_t.cmo \ $(top_builddir)/mllib/common_utils.cmo \ $(top_builddir)/mllib/JSON.cmo \ xml.cmo \ -- 2.7.4
Richard W.M. Jones
2016-Jun-02 15:11 UTC
[Libguestfs] [PATCH 3/3] builder: Warn if --output is a host partition.
But allow the warning to be turned off using --no-warn-if-partition.
Ming Xie tried to create a bootable USB key using
'virt-p2v-make-disk -o /dev/sdX1'. That works, but doesn't create a
bootable key because it puts everything into the first partition.
Emit a warning if someone tries to do this to try to prevent user
error:
virt-builder: warning: output device (/dev/sdb1) is a partition. If you
are writing to a USB key or external drive then you probably need to write
to the whole device, not to a partition. If this warning is wrong then you
can disable it with --no-warn-if-partition
---
builder/builder.ml | 9 +++++++++
builder/cmdline.ml | 6 ++++++
builder/cmdline.mli | 1 +
builder/virt-builder.pod | 9 +++++++++
4 files changed, 25 insertions(+)
diff --git a/builder/builder.ml b/builder/builder.ml
index 3c17ccd..affce10 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -280,6 +280,15 @@ let main ()
(* --- If we get here, we want to create a guest. --- *)
+ (* Warn if the user might be writing to a partition on a USB key. *)
+ (match cmdline.output with
+ | Some device when is_partition device ->
+ if cmdline.warn_if_partition then
+ warning (f_"output device (%s) is a partition. If you are writing
to a USB key or external drive then you probably need to write to the whole
device, not to a partition. If this warning is wrong then you can disable it
with --no-warn-if-partition")
+ device;
+ | Some _ | None -> ()
+ );
+
(* Download the template, or it may be in the cache. *)
let template let template, delete_on_exit diff --git
a/builder/cmdline.ml b/builder/cmdline.ml
index f872bd4..7b33013 100644
--- a/builder/cmdline.ml
+++ b/builder/cmdline.ml
@@ -51,6 +51,7 @@ type cmdline = {
smp : int option;
sources : (string * string) list;
sync : bool;
+ warn_if_partition : bool;
}
let parse_cmdline () @@ -115,6 +116,7 @@ let parse_cmdline () let add_source
arg = sources := arg :: !sources in
let sync = ref true in
+ let warn_if_partition = ref true in
let argspec = [
"--arch", Arg.Set_string arch, "arch" ^ "
" ^ s_"Set the output architecture";
@@ -163,6 +165,8 @@ let parse_cmdline () "--smp", Arg.Int
set_smp, "vcpus" ^ " " ^ s_"Set number of
vCPUs";
"--source", Arg.String add_source, "URL" ^ "
" ^ s_"Set source URL";
"--no-sync", Arg.Clear sync, " " ^
s_"Do not fsync output file on exit";
+ "--no-warn-if-partition", Arg.Clear warn_if_partition,
+ " " ^ s_"Do not warn
if writing to a partition";
] in
let customize_argspec, get_customize_ops = Customize_cmdline.argspec () in
let customize_argspec @@ -212,6 +216,7 @@ read the man page virt-builder(1).
let smp = !smp in
let sources = List.rev !sources in
let sync = !sync in
+ let warn_if_partition = !warn_if_partition in
(* No arguments and machine-readable mode? Print some facts. *)
if args = [] && machine_readable then (
@@ -324,4 +329,5 @@ read the man page virt-builder(1).
gpg = gpg; list_format = list_format; memsize = memsize;
network = network; ops = ops; output = output;
size = size; smp = smp; sources = sources; sync = sync;
+ warn_if_partition = warn_if_partition;
}
diff --git a/builder/cmdline.mli b/builder/cmdline.mli
index 4c201dd..854db61 100644
--- a/builder/cmdline.mli
+++ b/builder/cmdline.mli
@@ -39,6 +39,7 @@ type cmdline = {
smp : int option;
sources : (string * string) list;
sync : bool;
+ warn_if_partition : bool;
}
val parse_cmdline : unit -> cmdline
diff --git a/builder/virt-builder.pod b/builder/virt-builder.pod
index be5b568..cd3be9d 100644
--- a/builder/virt-builder.pod
+++ b/builder/virt-builder.pod
@@ -496,6 +496,15 @@ Note that you should not point I<--source> to sources
that you don't
trust (unless the source is signed by someone you do trust). See also
the I<--no-network> option.
+=item B<--no-warn-if-partition>
+
+Do not emit a warning if the output device is a partition. This
+warning avoids a common user error when writing to a USB key or
+external drive, when you should normally write to the whole device
+(S<I<--output /dev/sdX>>), not to a partition on the device
+(S<I<--output /dev/sdX1>>). Use this option to I<suppress>
this
+warning.
+
=item B<-v>
=item B<--verbose>
--
2.7.4