10:11 a.m.
Rather complex patch to solve a small user error. Warn if the
user is doing something like: virt-builder -o /dev/sdX1
Rich.
10:11 a.m.
New subject: [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
10:11 a.m.
New subject: [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
10:11 a.m.
New subject: [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
3095
days inactive
3095
days old
3 comments
1 participants
participants (1)
-
Richard W.M. Jones