[Libguestfs] [libnbd PATCH 1/2] api: Better use of nbd_internal_reset_size_and_flags

Eric Blake eblake at redhat.com
Wed Aug 24 02:53:40 UTC 2022


The documentation for nbd_internal_reset_size_and_flags() claims we
should wipe all knowledge of previously-negotiated meta contexts when
swapping export names.  However, we weren't actually doing that, which
could lead to a user calling nbd_opt_info() for one export, then
switching to another export name, then having nbd_get_size() still
report the size of the old export.  At present, this is only mildly
confusing, but once the next patch exposes nbd_opt_set_meta_context()
for user control, it would become actively dangerous: negotiating meta
contexts for one export, then switching to a different export name
before nbd_opt_go, can lead us into calling nbd_cmd_block_status() when
the server is not expecting it.

While fixing this, code things to check whether we are actually
swapping export names; an idempotent nbd_set_export_name() is not
observable by the server, and therefore does not need to invalidate
what has already been negotiated.

Conversely, calling nbd_opt_list_meta_context() does not need to wipe
the list - it does not touch h->meta_contexts, but relies on the
user's callback instead.
---
 generator/states-newstyle-opt-meta-context.c | 7 +++----
 lib/handle.c                                 | 4 ++++
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/generator/states-newstyle-opt-meta-context.c b/generator/states-newstyle-opt-meta-context.c
index 30b9617..76f1032 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -1,5 +1,5 @@
 /* nbd client library in userspace: state machine
- * Copyright (C) 2013-2020 Red Hat Inc.
+ * Copyright (C) 2013-2022 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -28,7 +28,6 @@ STATE_MACHINE {
    * contexts, when doing SET (but an empty LIST is okay).
    */
   assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
-  nbd_internal_reset_size_and_flags (h);
   if (h->opt_current == NBD_OPT_LIST_META_CONTEXT) {
     assert (h->opt_mode);
     assert (h->structured_replies);
@@ -37,6 +36,8 @@ STATE_MACHINE {
   }
   else {
     assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
+    nbd_internal_reset_size_and_flags (h);
+    assert (h->meta_contexts == NULL);
     opt = NBD_OPT_SET_META_CONTEXT;
     if (!h->structured_replies || h->request_meta_contexts.len == 0) {
       SET_NEXT_STATE (%^OPT_GO.START);
@@ -44,8 +45,6 @@ STATE_MACHINE {
     }
   }

-  assert (h->meta_contexts == NULL);
-
   /* Calculate the length of the option request data. */
   len = 4 /* exportname len */ + strlen (h->export_name) + 4 /* nr queries */;
   for (i = 0; i < h->request_meta_contexts.len; ++i)
diff --git a/lib/handle.c b/lib/handle.c
index 8713e18..2aa89b9 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -219,6 +219,10 @@ nbd_unlocked_set_export_name (struct nbd_handle *h, const char *export_name)
     return -1;
   }

+  if (strcmp (export_name, h->export_name) == 0)
+    return 0;
+
+  nbd_internal_reset_size_and_flags (h);
   new_name = strdup (export_name);
   if (!new_name) {
     set_error (errno, "strdup");
-- 
2.37.2



More information about the Libguestfs mailing list