[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH 3/4] nbd/server: implement dirty bitmap export

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH 3/4] nbd/server: implement dirty bitmap export
Date: Thu, 22 Mar 2018 18:32:45 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

21.03.2018 19:57, Eric Blake wrote:
On 03/21/2018 07:19 AM, Vladimir Sementsov-Ogievskiy wrote:
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>


+#define NBD_STATE_DIRTY 1

Does this belong better in nbd.h, alongside NBD_STATE_HOLE?  (And defining it as (1 << 0) might be nicer, too).

It's not used anywhere else, but it may be worth moving, for consistency and for future users. Ok, I'll move.


  +/* nbd_meta_bitmap_query
+ *
+ * Handle query to 'qemu-dirty-bitmap' namespace.
+ * 'len' is the amount of text remaining to be read from the current name, after
+ * the 'qemu-dirty-bitmap:' portion has been stripped.
+ *
+ * Return -errno on I/O error, 0 if option was completely handled by
+ * sending a reply about inconsistent lengths, or 1 on success. */
+static int nbd_meta_bitmap_query(NBDClient *client, NBDExportMetaContexts *meta,
+                                 uint32_t len, Error **errp)
+    if (!client->exp->export_bitmap) {
+        return nbd_opt_skip(client, len, errp);
+    }
+    return nbd_meta_single_query(client, client->exp->export_bitmap_name, len,
+                                 &meta->dirty_bitmap, errp);

So if I'm reading this right, a client can do _LIST_ 'qemu-dirty-bitmap:' (or 'qemu:dirty-bitmap:' if we choose a shorter initial namespace) and get back the exported bitmap name; or the user already knows the bitmap name and both _LIST_ and _SET_ 'qemu-dirty-bitmap:name' return the one supported bitmap for that NBD server.


  /* nbd_co_send_extents
+ *
+ * NBD_REPLY_FLAG_DONE is not set, don't forget to send it.
+ *
   * @extents should be in big-endian */
  static int nbd_co_send_extents(NBDClient *client, uint64_t handle,
                                 NBDExtent *extents, unsigned nb_extents,

Worth a bool flag that the caller can inform this function whether to include FLAG_DONE?

It was simpler to just send it separately, after all BLOCK_STATUS replies. So, I didn't need it.

+static int nbd_co_send_bitmap(NBDClient *client, uint64_t handle,
+                              BdrvDirtyBitmap *bitmap, uint64_t offset,
+                              uint64_t length, uint32_t context_id,
+                              Error **errp)
+    int ret;
+    unsigned nb_extents;
+    NBDExtent *extents = g_new(NBDExtent, NBD_MAX_BITMAP_EXTENTS);

Is this correct even when the client used NBD_CMD_FLAG_REQ_ONE?

Oops, looks like a bug.

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]