[PATCH v2 0/2] lib: Allow db_dump package to be a weak dependency
by Richard W.M. Jones
Previously posted:
https://www.redhat.com/archives/libguestfs/2017-October/msg00032.html
This takes a completely different approach. It turns out that POSIX /
the shell already defines a special exit code 127 for ‘command not
found’. We can make a small adjustment to lib/command.c to return
this exit code in that case.
Then we just have to modify the db_dump code to test for this exit
code.
I also made db_dump unconditional, allowing the packager to either
build without db_dump present at all, and/or to make db_dump into a
weak runtime dependency which will be silently ignored if not present
by the mechanism above.
Rich.
6 years, 6 months
[PATCH 1/2] common/mlstdutils: Add with_open_in and with_open_out functions.
by Richard W.M. Jones
These safe wrappers around Pervasives.open_in and Pervasives.open_out
ensure that exceptions escaping cannot leave unclosed files.
---
common/mlstdutils/std_utils.ml | 39 ++++++++++++++++++++--------------
common/mlstdutils/std_utils.mli | 12 +++++++++++
common/mltools/tools_utils.ml | 39 +++++++++++++++++-----------------
dib/dib.ml | 9 ++++----
generator/bindtests.ml | 26 ++++++++++++-----------
generator/utils.ml | 14 ++++---------
v2v/changeuid.ml | 7 +------
v2v/copy_to_local.ml | 4 +---
v2v/input_libvirt_vddk.ml | 9 ++++----
v2v/input_ova.ml | 46 +++++++++++++++++++++--------------------
v2v/output_local.ml | 4 +---
v2v/output_qemu.ml | 29 +++++++++++++-------------
v2v/output_vdsm.ml | 8 ++-----
13 files changed, 127 insertions(+), 119 deletions(-)
diff --git a/common/mlstdutils/std_utils.ml b/common/mlstdutils/std_utils.ml
index ba23f39ed..ee6bea5af 100644
--- a/common/mlstdutils/std_utils.ml
+++ b/common/mlstdutils/std_utils.ml
@@ -654,20 +654,29 @@ let verbose = ref false
let set_verbose () = verbose := true
let verbose () = !verbose
+let with_open_in filename f =
+ let chan = open_in filename in
+ protect ~f:(fun () -> f chan) ~finally:(fun () -> close_in chan)
+
+let with_open_out filename f =
+ let chan = open_out filename in
+ protect ~f:(fun () -> f chan) ~finally:(fun () -> close_out chan)
+
let read_whole_file path =
let buf = Buffer.create 16384 in
- let chan = open_in path in
- let maxlen = 16384 in
- let b = Bytes.create maxlen in
- let rec loop () =
- let r = input chan b 0 maxlen in
- if r > 0 then (
- Buffer.add_substring buf (Bytes.to_string b) 0 r;
+ with_open_in path (
+ fun chan ->
+ let maxlen = 16384 in
+ let b = Bytes.create maxlen in
+ let rec loop () =
+ let r = input chan b 0 maxlen in
+ if r > 0 then (
+ Buffer.add_substring buf (Bytes.to_string b) 0 r;
+ loop ()
+ )
+ in
loop ()
- )
- in
- loop ();
- close_in chan;
+ );
Buffer.contents buf
(* Compare two version strings intelligently. *)
@@ -824,10 +833,10 @@ let last_part_of str sep =
with Not_found -> None
let read_first_line_from_file filename =
- let chan = open_in filename in
- let line = try input_line chan with End_of_file -> "" in
- close_in chan;
- line
+ with_open_in filename (
+ fun chan ->
+ try input_line chan with End_of_file -> ""
+ )
let is_regular_file path = (* NB: follows symlinks. *)
try (Unix.stat path).Unix.st_kind = Unix.S_REG
diff --git a/common/mlstdutils/std_utils.mli b/common/mlstdutils/std_utils.mli
index 96c55a511..7af6c2111 100644
--- a/common/mlstdutils/std_utils.mli
+++ b/common/mlstdutils/std_utils.mli
@@ -387,6 +387,18 @@ val verbose : unit -> bool
(** Stores the colours ([--colours]), quiet ([--quiet]), trace ([-x])
and verbose ([-v]) flags in global variables. *)
+val with_open_in : string -> (in_channel -> 'a) -> 'a
+(** [with_open_in filename f] calls function [f] with [filename]
+ open for input. The file is always closed either on normal
+ return or if the function [f] throws an exception, so this is
+ both safer and more concise than the regular function. *)
+
+val with_open_out : string -> (out_channel -> 'a) -> 'a
+(** [with_open_out filename f] calls function [f] with [filename]
+ open for output. The file is always closed either on normal
+ return or if the function [f] throws an exception, so this is
+ both safer and more concise than the regular function. *)
+
val read_whole_file : string -> string
(** Read in the whole file as a string. *)
diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
index 8140ba84d..95658a75f 100644
--- a/common/mltools/tools_utils.ml
+++ b/common/mltools/tools_utils.ml
@@ -478,26 +478,25 @@ let debug_augeas_errors g =
(* Detect type of a file. *)
let detect_file_type filename =
- let chan = open_in filename in
- let get start size =
- try
- seek_in chan start;
- let b = Bytes.create size in
- really_input chan b 0 size;
- Some (Bytes.to_string b)
- with End_of_file | Invalid_argument _ -> None
- in
- let ret =
- if get 0 6 = Some "\2537zXZ\000" then `XZ
- else if get 0 4 = Some "PK\003\004" then `Zip
- else if get 0 4 = Some "PK\005\006" then `Zip
- else if get 0 4 = Some "PK\007\008" then `Zip
- else if get 257 6 = Some "ustar\000" then `Tar
- else if get 257 8 = Some "ustar\x20\x20\000" then `Tar
- else if get 0 2 = Some "\x1f\x8b" then `GZip
- else `Unknown in
- close_in chan;
- ret
+ with_open_in filename (
+ fun chan ->
+ let get start size =
+ try
+ seek_in chan start;
+ let b = Bytes.create size in
+ really_input chan b 0 size;
+ Some (Bytes.to_string b)
+ with End_of_file | Invalid_argument _ -> None
+ in
+ if get 0 6 = Some "\2537zXZ\000" then `XZ
+ else if get 0 4 = Some "PK\003\004" then `Zip
+ else if get 0 4 = Some "PK\005\006" then `Zip
+ else if get 0 4 = Some "PK\007\008" then `Zip
+ else if get 257 6 = Some "ustar\000" then `Tar
+ else if get 257 8 = Some "ustar\x20\x20\000" then `Tar
+ else if get 0 2 = Some "\x1f\x8b" then `GZip
+ else `Unknown
+ )
let is_partition dev =
try
diff --git a/dib/dib.ml b/dib/dib.ml
index 9a8d86bd9..94ad3003a 100644
--- a/dib/dib.ml
+++ b/dib/dib.ml
@@ -60,10 +60,11 @@ let read_dib_envvars () =
String.concat "" vars
let write_script fn text =
- let oc = open_out fn in
- output_string oc text;
- flush oc;
- close_out oc;
+ with_open_out fn (
+ fun oc ->
+ output_string oc text;
+ flush oc
+ );
Unix.chmod fn 0o755
let envvars_string l =
diff --git a/generator/bindtests.ml b/generator/bindtests.ml
index 4bdff8092..79b020326 100644
--- a/generator/bindtests.ml
+++ b/generator/bindtests.ml
@@ -966,18 +966,20 @@ and generate_php_bindtests () =
pr "--EXPECT--\n";
let dump filename =
- let chan = open_in filename in
- let rec loop () =
- let line = input_line chan in
- (match String.nsplit ":" line with
- | ("obool"|"oint"|"oint64"|"ostring"|"ostringlist") as x :: _ ->
- pr "%s: unset\n" x
- | _ -> pr "%s\n" line
- );
- loop ()
- in
- (try loop () with End_of_file -> ());
- close_in chan in
+ with_open_in filename (
+ fun chan ->
+ let rec loop () =
+ let line = input_line chan in
+ (match String.nsplit ":" line with
+ | ("obool"|"oint"|"oint64"|"ostring"|"ostringlist") as x :: _ ->
+ pr "%s: unset\n" x
+ | _ -> pr "%s\n" line
+ );
+ loop ()
+ in
+ (try loop () with End_of_file -> ());
+ )
+ in
dump "bindtests"
diff --git a/generator/utils.ml b/generator/utils.ml
index b818a0b3c..e91fed577 100644
--- a/generator/utils.ml
+++ b/generator/utils.ml
@@ -179,19 +179,13 @@ type memo_value = string list (* list of lines of POD file *)
let pod2text_memo_filename = "generator/.pod2text.data.version.2"
let pod2text_memo : (memo_key, memo_value) Hashtbl.t =
- try
- let chan = open_in pod2text_memo_filename in
- let v = input_value chan in
- close_in chan;
- v
- with
- _ -> Hashtbl.create 13
+ try with_open_in pod2text_memo_filename input_value
+ with _ -> Hashtbl.create 13
let pod2text_memo_unsaved_count = ref 0
let pod2text_memo_atexit = ref false
let pod2text_memo_save () =
- let chan = open_out pod2text_memo_filename in
- output_value chan pod2text_memo;
- close_out chan
+ with_open_out pod2text_memo_filename
+ (fun chan -> output_value chan pod2text_memo)
let pod2text_memo_updated () =
if not (!pod2text_memo_atexit) then (
at_exit pod2text_memo_save;
diff --git a/v2v/changeuid.ml b/v2v/changeuid.ml
index 49290c298..f4c5c90d1 100644
--- a/v2v/changeuid.ml
+++ b/v2v/changeuid.ml
@@ -66,12 +66,7 @@ let rmdir t path =
with_fork t (sprintf "rmdir: %s" path) (fun () -> rmdir path)
let output t path f =
- with_fork t path (
- fun () ->
- let chan = open_out path in
- f chan;
- close_out chan
- )
+ with_fork t path (fun () -> with_open_out path f)
let make_file t path content =
output t path (fun chan -> output_string chan content)
diff --git a/v2v/copy_to_local.ml b/v2v/copy_to_local.ml
index f1a67fc14..3e41016c5 100644
--- a/v2v/copy_to_local.ml
+++ b/v2v/copy_to_local.ml
@@ -226,9 +226,7 @@ read the man page virt-v2v-copy-to-local(1).
let guest_xml = guest_name ^ ".xml" in
message (f_"Writing libvirt XML metadata to %s ...") guest_xml;
- let chan = open_out guest_xml in
- output_string chan xml;
- close_out chan;
+ with_open_out guest_xml (fun chan -> output_string chan xml);
(* Finished, so don't delete the disks on exit. *)
message (f_"Finishing off");
diff --git a/v2v/input_libvirt_vddk.ml b/v2v/input_libvirt_vddk.ml
index 63e76a5aa..e29fbc2b7 100644
--- a/v2v/input_libvirt_vddk.ml
+++ b/v2v/input_libvirt_vddk.ml
@@ -240,10 +240,11 @@ object
"password=-"
| Some password ->
let password_file = tmpdir // "password" in
- let chan = open_out password_file in
- chmod password_file 0o600;
- output_string chan password;
- close_out chan;
+ with_open_out password_file (
+ fun chan ->
+ chmod password_file 0o600;
+ output_string chan password
+ );
(* nbdkit reads the password from the file *)
"password=+" ^ password_file in
add_arg (sprintf "server=%s" server);
diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index abb0654a5..ff00118b3 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -215,29 +215,31 @@ object
debug "processing manifest %s" mf;
let mf_folder = Filename.dirname mf in
let mf_subfolder = subdirectory exploded mf_folder in
- let chan = open_in mf in
- let rec loop () =
- let line = input_line chan in
- if PCRE.matches rex line then (
- let mode = PCRE.sub 1
- and disk = PCRE.sub 2
- and expected = PCRE.sub 3 in
- let csum = Checksums.of_string mode expected in
- try
- if partial then
- Checksums.verify_checksum csum ~tar:ova (mf_subfolder // disk)
+ with_open_in mf (
+ fun chan ->
+ let rec loop () =
+ let line = input_line chan in
+ if PCRE.matches rex line then (
+ let mode = PCRE.sub 1
+ and disk = PCRE.sub 2
+ and expected = PCRE.sub 3 in
+ let csum = Checksums.of_string mode expected in
+ try
+ if partial then
+ Checksums.verify_checksum csum
+ ~tar:ova (mf_subfolder // disk)
+ else
+ Checksums.verify_checksum csum (mf_folder // disk)
+ with Checksums.Mismatched_checksum (_, actual) ->
+ error (f_"checksum of disk %s does not match manifest %s (actual %s(%s) = %s, expected %s(%s) = %s)")
+ disk mf mode disk actual mode disk expected;
+ )
else
- Checksums.verify_checksum csum (mf_folder // disk)
- with Checksums.Mismatched_checksum (_, actual) ->
- error (f_"checksum of disk %s does not match manifest %s (actual %s(%s) = %s, expected %s(%s) = %s)")
- disk mf mode disk actual mode disk expected;
- )
- else
- warning (f_"unable to parse line from manifest file: %S") line;
- loop ()
- in
- (try loop () with End_of_file -> ());
- close_in chan
+ warning (f_"unable to parse line from manifest file: %S") line;
+ loop ()
+ in
+ (try loop () with End_of_file -> ())
+ )
) mf;
let ovf_folder = Filename.dirname ovf in
diff --git a/v2v/output_local.ml b/v2v/output_local.ml
index 93d643f03..97ad8dddd 100644
--- a/v2v/output_local.ml
+++ b/v2v/output_local.ml
@@ -67,9 +67,7 @@ class output_local dir = object
let name = source.s_name in
let file = dir // name ^ ".xml" in
- let chan = open_out file in
- DOM.doc_to_chan chan doc;
- close_out chan
+ with_open_out file (fun chan -> DOM.doc_to_chan chan doc)
end
let output_local = new output_local
diff --git a/v2v/output_qemu.ml b/v2v/output_qemu.ml
index 5304329ae..f61d698d6 100644
--- a/v2v/output_qemu.ml
+++ b/v2v/output_qemu.ml
@@ -229,23 +229,24 @@ object
arg "-serial" "stdio";
(* Write the output file. *)
- let chan = open_out file in
- let fpf fs = fprintf chan fs in
- fpf "#!/bin/sh -\n";
- fpf "\n";
+ with_open_out file (
+ fun chan ->
+ let fpf fs = fprintf chan fs in
+ fpf "#!/bin/sh -\n";
+ fpf "\n";
- (match uefi_firmware with
- | None -> ()
- | Some { Uefi.vars = vars_template } ->
- fpf "# Make a copy of the UEFI variables template\n";
- fpf "uefi_vars=\"$(mktemp)\"\n";
- fpf "cp %s \"$uefi_vars\"\n" (quote vars_template);
- fpf "\n"
+ (match uefi_firmware with
+ | None -> ()
+ | Some { Uefi.vars = vars_template } ->
+ fpf "# Make a copy of the UEFI variables template\n";
+ fpf "uefi_vars=\"$(mktemp)\"\n";
+ fpf "cp %s \"$uefi_vars\"\n" (quote vars_template);
+ fpf "\n"
+ );
+
+ Qemuopts.to_chan cmd chan
);
- Qemuopts.to_chan cmd chan;
- close_out chan;
-
Unix.chmod file 0o755;
(* If --qemu-boot option was specified then we should boot the guest. *)
diff --git a/v2v/output_vdsm.ml b/v2v/output_vdsm.ml
index 0aeee289d..d5911e80e 100644
--- a/v2v/output_vdsm.ml
+++ b/v2v/output_vdsm.ml
@@ -144,9 +144,7 @@ object
List.iter (
fun ({ target_file }, meta) ->
let meta_filename = target_file ^ ".meta" in
- let chan = open_out meta_filename in
- output_string chan meta;
- close_out chan
+ with_open_out meta_filename (fun chan -> output_string chan meta)
) (List.combine targets metas);
(* Return the list of targets. *)
@@ -177,9 +175,7 @@ object
(* Write it to the metadata file. *)
let file = vdsm_params.ovf_output // vdsm_params.vm_uuid ^ ".ovf" in
- let chan = open_out file in
- DOM.doc_to_chan chan ovf;
- close_out chan
+ with_open_out file (fun chan -> DOM.doc_to_chan chan ovf)
end
let output_vdsm = new output_vdsm
--
2.13.2
6 years, 6 months
[nbdkit PATCH] connections: Extract common export flag computation code
by Eric Blake
No need to duplicate maintenance of export flag computation between
old and new style handshakes.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/connections.c | 120 +++++++++++++++++++++---------------------------------
1 file changed, 47 insertions(+), 73 deletions(-)
diff --git a/src/connections.c b/src/connections.c
index 8dc1925..f9edea7 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -237,13 +237,57 @@ free_connection (struct connection *conn)
}
static int
+compute_eflags (struct connection *conn, uint16_t *flags)
+{
+ uint16_t eflags = NBD_FLAG_HAS_FLAGS;
+ int fl;
+
+ fl = plugin_can_write (conn);
+ if (fl == -1)
+ return -1;
+ if (readonly || !fl) {
+ eflags |= NBD_FLAG_READ_ONLY;
+ conn->readonly = 1;
+ }
+ if (!conn->readonly) {
+ eflags |= NBD_FLAG_SEND_WRITE_ZEROES;
+ }
+
+ fl = plugin_can_flush (conn);
+ if (fl == -1)
+ return -1;
+ if (fl) {
+ eflags |= NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA;
+ conn->can_flush = 1;
+ }
+
+ fl = plugin_is_rotational (conn);
+ if (fl == -1)
+ return -1;
+ if (fl) {
+ eflags |= NBD_FLAG_ROTATIONAL;
+ conn->is_rotational = 1;
+ }
+
+ fl = plugin_can_trim (conn);
+ if (fl == -1)
+ return -1;
+ if (fl) {
+ eflags |= NBD_FLAG_SEND_TRIM;
+ conn->can_trim = 1;
+ }
+
+ *flags = eflags;
+ return 0;
+}
+
+static int
_negotiate_handshake_oldstyle (struct connection *conn)
{
struct old_handshake handshake;
int64_t r;
uint64_t exportsize;
uint16_t gflags, eflags;
- int fl;
/* In --tls=require / FORCEDTLS mode, old style handshakes are
* rejected because they cannot support TLS.
@@ -265,43 +309,8 @@ _negotiate_handshake_oldstyle (struct connection *conn)
conn->exportsize = exportsize;
gflags = 0;
- eflags = NBD_FLAG_HAS_FLAGS;
-
- fl = plugin_can_write (conn);
- if (fl == -1)
+ if (compute_eflags (conn, &eflags) < 0)
return -1;
- if (readonly || !fl) {
- eflags |= NBD_FLAG_READ_ONLY;
- conn->readonly = 1;
- }
- if (!conn->readonly) {
- eflags |= NBD_FLAG_SEND_WRITE_ZEROES;
- }
-
-
- fl = plugin_can_flush (conn);
- if (fl == -1)
- return -1;
- if (fl) {
- eflags |= NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA;
- conn->can_flush = 1;
- }
-
- fl = plugin_is_rotational (conn);
- if (fl == -1)
- return -1;
- if (fl) {
- eflags |= NBD_FLAG_ROTATIONAL;
- conn->is_rotational = 1;
- }
-
- fl = plugin_can_trim (conn);
- if (fl == -1)
- return -1;
- if (fl) {
- eflags |= NBD_FLAG_SEND_TRIM;
- conn->can_trim = 1;
- }
debug ("oldstyle negotiation: flags: global 0x%x export 0x%x",
gflags, eflags);
@@ -552,7 +561,6 @@ _negotiate_handshake_newstyle (struct connection *conn)
int64_t r;
uint64_t exportsize;
uint16_t eflags;
- int fl;
gflags = NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES;
@@ -596,42 +604,8 @@ _negotiate_handshake_newstyle (struct connection *conn)
exportsize = (uint64_t) r;
conn->exportsize = exportsize;
- eflags = NBD_FLAG_HAS_FLAGS;
-
- fl = plugin_can_write (conn);
- if (fl == -1)
+ if (compute_eflags (conn, &eflags) < 0)
return -1;
- if (readonly || !fl) {
- eflags |= NBD_FLAG_READ_ONLY;
- conn->readonly = 1;
- }
- if (!conn->readonly) {
- eflags |= NBD_FLAG_SEND_WRITE_ZEROES;
- }
-
- fl = plugin_can_flush (conn);
- if (fl == -1)
- return -1;
- if (fl) {
- eflags |= NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA;
- conn->can_flush = 1;
- }
-
- fl = plugin_is_rotational (conn);
- if (fl == -1)
- return -1;
- if (fl) {
- eflags |= NBD_FLAG_ROTATIONAL;
- conn->is_rotational = 1;
- }
-
- fl = plugin_can_trim (conn);
- if (fl == -1)
- return -1;
- if (fl) {
- eflags |= NBD_FLAG_SEND_TRIM;
- conn->can_trim = 1;
- }
debug ("newstyle negotiation: flags: export 0x%x", eflags);
--
2.13.6
6 years, 6 months
[nbdkit PATCH] connections: Improve error responses
by Eric Blake
We had several inconsistencies from the NBD spec when diagnosing
bad client messages:
- FLUSH is not generally forbidden on a read-only export (so failing
with EPERM is wrong) [meanwhile, if we don't advertise flush because
plugin_can_flush() fails, then rejecting with EINVAL is still okay]
- returning EPERM (aka EROFS) for read-only exports should probably
take precedence over anything else
- out-of-bounds WRITE and WRITE_ZEROES should fail with ENOSPC,
not EIO
- out-of-bounds READ and TRIM should fail with EINVAL, not EIO
We also had dead code: valid_range() and validate_request() cannot
return -1. Make these functions return bool instead. And finally,
fix a comment typo.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/connections.c | 53 ++++++++++++++++++++++++-----------------------------
1 file changed, 24 insertions(+), 29 deletions(-)
diff --git a/src/connections.c b/src/connections.c
index 8dc1925..264037d 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -666,7 +666,7 @@ negotiate_handshake (struct connection *conn)
return r;
}
-static int
+static bool
valid_range (struct connection *conn, uint64_t offset, uint32_t count)
{
uint64_t exportsize = conn->exportsize;
@@ -674,27 +674,34 @@ valid_range (struct connection *conn, uint64_t offset, uint32_t count)
return count > 0 && offset <= exportsize && offset + count <= exportsize;
}
-static int
+static bool
validate_request (struct connection *conn,
uint32_t cmd, uint32_t flags, uint64_t offset, uint32_t count,
uint32_t *error)
{
int r;
+ /* Readonly connection? */
+ if (conn->readonly &&
+ (cmd == NBD_CMD_WRITE ||
+ cmd == NBD_CMD_TRIM || cmd == NBD_CMD_WRITE_ZEROES)) {
+ nbdkit_error ("invalid request: write request on readonly connection");
+ *error = EROFS;
+ return false;
+ }
+
/* Validate cmd, offset, count. */
switch (cmd) {
case NBD_CMD_READ:
case NBD_CMD_WRITE:
case NBD_CMD_TRIM:
case NBD_CMD_WRITE_ZEROES:
- r = valid_range (conn, offset, count);
- if (r == -1)
- return -1;
- if (r == 0) {
+ if (!valid_range (conn, offset, count)) {
/* XXX Allow writes to extend the disk? */
nbdkit_error ("invalid request: offset and length are out of range");
- *error = EIO;
- return 0;
+ *error = (cmd == NBD_CMD_WRITE ||
+ cmd == NBD_CMD_WRITE_ZEROES) ? ENOSPC : EINVAL;
+ return false;
}
break;
@@ -702,7 +709,7 @@ validate_request (struct connection *conn,
if (offset != 0 || count != 0) {
nbdkit_error ("invalid flush request: expecting offset and length == 0");
*error = EINVAL;
- return 0;
+ return false;
}
break;
@@ -710,20 +717,20 @@ validate_request (struct connection *conn,
nbdkit_error ("invalid request: unknown command (%" PRIu32 ") ignored",
cmd);
*error = EINVAL;
- return 0;
+ return false;
}
/* Validate flags */
if (flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE)) {
nbdkit_error ("invalid request: unknown flag (0x%x)", flags);
*error = EINVAL;
- return 0;
+ return false;
}
if ((flags & NBD_CMD_FLAG_NO_HOLE) &&
cmd != NBD_CMD_WRITE_ZEROES) {
nbdkit_error ("invalid request: NO_HOLE flag needs WRITE_ZEROES request");
*error = EINVAL;
- return 0;
+ return false;
}
/* Refuse over-large read and write requests. */
@@ -732,33 +739,24 @@ validate_request (struct connection *conn,
nbdkit_error ("invalid request: data request is too large (%" PRIu32
" > %d)", count, MAX_REQUEST_SIZE);
*error = ENOMEM;
- return 0;
- }
-
- /* Readonly connection? */
- if (conn->readonly &&
- (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_FLUSH ||
- cmd == NBD_CMD_TRIM || cmd == NBD_CMD_WRITE_ZEROES)) {
- nbdkit_error ("invalid request: write request on readonly connection");
- *error = EROFS;
- return 0;
+ return false;
}
/* Flush allowed? */
if (!conn->can_flush && cmd == NBD_CMD_FLUSH) {
nbdkit_error ("invalid request: flush operation not supported");
*error = EINVAL;
- return 0;
+ return false;
}
/* Trim allowed? */
if (!conn->can_trim && cmd == NBD_CMD_TRIM) {
nbdkit_error ("invalid request: trim operation not supported");
*error = EINVAL;
- return 0;
+ return false;
}
- return 1; /* Commands validates. */
+ return true; /* Command validates. */
}
/* Grab the appropriate error value.
@@ -970,10 +968,7 @@ recv_request_send_reply (struct connection *conn)
}
/* Validate the request. */
- r = validate_request (conn, cmd, flags, offset, count, &error);
- if (r == -1)
- return -1;
- if (r == 0) { /* request not valid */
+ if (!validate_request (conn, cmd, flags, offset, count, &error)) {
if (cmd == NBD_CMD_WRITE &&
skip_over_write_buffer (conn->sockin, count) < 0)
return -1;
--
2.13.6
6 years, 6 months
[nbdkit PATCH 0/2] Better response to bogus NBD_CMD_READ
by Eric Blake
When facing a malicious client that is sending bogus NBD_CMD_READ,
we should make sure that we never end up in a situation where we
could try to treat the tail from a command that we diagnosed as
bad as being further commands.
Eric Blake (2):
connections: Report mid-message EOF as fatal
connections: Hang up early on insanely large WRITE requests
src/connections.c | 35 +++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 12 deletions(-)
--
2.13.6
6 years, 6 months
Re: [Libguestfs] [Qemu-devel] [qemu-img] support for XVA
by Richard W.M. Jones
On Wed, Nov 15, 2017 at 04:52:56PM +0100, Gandalf Corvotempesta wrote:
> I'm thinking if support for XVA files could be added to qemu-img
> The file-format is well known (it's just a tar archive) and there are scripts
> that are able to convert an XVA file to a RAW image. (ie:
> https://gist.github.com/miebach/0433947bcf053de23159)
>
> Running these script on their own is very time consuming, as you have to
> extract the XVA, convert any disk image from "single chunks" to a raw image
> and then use qemu-img to convert from raw to qcow.
>
> Maybe a native support will be able to skip some steps. (like the
> conversion from
> "single chunks" to raw and raw to qcow2)
Why not use the offset/size support in the raw driver?
We use that to access files inside OVA files (also a tar file with a
funny extension) in virt-v2v:
https://github.com/libguestfs/libguestfs/blob/994ca8d87361a4eb52a05bb4496...
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
6 years, 6 months
Re: [Libguestfs] [Qemu-devel] [qemu-img] support for XVA
by Eric Blake
[adding libguestfs]
On 11/15/2017 09:52 AM, Gandalf Corvotempesta wrote:
> I'm thinking if support for XVA files could be added to qemu-img
> The file-format is well known (it's just a tar archive) and there are scripts
> that are able to convert an XVA file to a RAW image. (ie:
> https://gist.github.com/miebach/0433947bcf053de23159)
>
> Running these script on their own is very time consuming, as you have to
> extract the XVA, convert any disk image from "single chunks" to a raw image
> and then use qemu-img to convert from raw to qcow.
>
> Maybe a native support will be able to skip some steps. (like the
> conversion from
> "single chunks" to raw and raw to qcow2)
Another possibility might be writing an nbdkit plugin that can directly
read XVA, then you can connect qemu to the NBD server provided by
nbdkit, without having to teach qemu proper how to read the file.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
6 years, 6 months
question on nbdkit shutdown behavior
by Eric Blake
I'm observing a difference in timing on nbdkit shutdown that is
dependent on client behavior, and I wonder if that's a bug, but I can't
figure out where to patch it.
When there is no connection present, sending SIGINT to nbdkit terminates
immediately.
If, on the other hand, there is a single client currently connected, the
termination behavior on SIGINT depends on what the client has done: if
the client is currently silent with regards to issuing
transmission-phase transactions, nbdkit hangs for 5 seconds, then
forcibly tears down the connection:
term1$ ./nbdkit -f -v -n file file=TODO
...
nbdkit: debug: bound to IP address <any>:10809 (2 socket(s))
term2$ qemu-io -f raw nbd://localhost:10809/ -r
qemu-io>
term1$ ^C
nbdkit: debug: waiting for running threads to complete
nbdkit: debug: waited 5s for running threads to complete
nbdkit: debug:
/home/eblake/nbdkit/plugins/file/.libs/nbdkit-file-plugin.so: unload
term2$
qemu-io> r 0 1
read failed: Input/output error
qemu-io> q
But, if the client makes ANY transaction during that window, then nbdkit
lets that transaction succeed, then tears down the connection without
any further wait:
term1$ ./nbdkit -f -v -n file file=TODO
...
nbdkit: debug: bound to IP address <any>:10809 (2 socket(s))
term2$ qemu-io -f raw nbd://localhost:10809/ -r
qemu-io> r 0 1 # wait to hit Enter until right after ^C in term1
term1$ ^C
nbdkit: debug: waiting for running threads to complete
term2$ # hit enter
term1$
nbdkit: file[1]: debug: acquire per-connection request lock
nbdkit: file[1]: debug: pread count=1 offset=0
nbdkit: file[1]: debug: release per-connection request lock
nbdkit: file[1]: debug: close
nbdkit: debug: waited 2s for running threads to complete
nbdkit: debug:
/home/eblake/nbdkit/plugins/file/.libs/nbdkit-file-plugin.so: unload
term2$
qemu-io> r 0 1
read 1/1 bytes at offset 0
1 bytes, 1 ops; 0.0002 sec (4.138 KiB/sec and 4237.2881 ops/sec)
Why does current traffic from the client cause the plugin to be torn
down faster? Does it matter?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
6 years, 6 months