Re: [Xen-devel] [PATCH 2 of 6 RESENT] libxl: Accept disk name in libxl_devid_to_device_disk

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Sun, 2011-06-05 at 17:50 +0100, Marek Marczykowski wrote:
> # HG changeset patch
> # User Marek Marczykowski <marmarek@xxxxxxxxxxxx>
> # Date 1306962929 -7200
> # Node ID 5231fa19f3e39091ff29e2a6dca057ca54403092
> # Parent  c32797243a6ba61dd2942a0307151e42fb7bf157
> libxl: Accept disk name in libxl_devid_to_device_disk
> 
> Accept disk name in xl block-detach.
> 
> Signed-off-by: Marek Marczykowski <marmarek@xxxxxxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

While reviewing I noticed a path out of libxl__device_disk_dev_number
which would succeed without setting pdisk or ppartition, which would
surprise a caller which provided them. 

I wasn't immediately sure from docs/misc/vbd-interface.txt how to parse
a vdev which is an arbitrary number. Many cases are covered in the
document but there are others (e.g. NetBSD uses small integers iirc)
which I was unsure of. (I expect that document needs expanding to cover
these cases, but I'm not sure what to put...)

For now I just returned an error to prevent a cascading failure.

8<-------------------------------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# Date 1307359172 -3600
# Node ID b5dfb12aa231c98a4dcf3372d3bf1cf9a85df2a4
# Parent  86009542df09c70ca8b4a95e4dd3de63cf95c9d6
libxl: fail to parse disk vpath if a disk+part number is required but 
unavailable

libxl__device_disk_dev_number() can parse a virtpath which is an encoded
unsigned long but does not set *pdisk or *ppartition in that case.

Ideally we would parse the number but for now simply fail to prevent cascading
failures.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

diff -r 86009542df09 -r b5dfb12aa231 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c        Mon Jun 06 12:11:25 2011 +0100
+++ b/tools/libxl/libxl_device.c        Mon Jun 06 12:19:32 2011 +0100
@@ -222,8 +222,12 @@ int libxl__device_disk_dev_number(char *
 
     errno = 0;
     ul = strtoul(virtpath, &ep, 0);
-    if (!errno && !*ep && ul <= INT_MAX)
+    if (!errno && !*ep && ul <= INT_MAX) {
+        /* FIXME: should parse ul to determine these. */
+        if (pdisk || ppartition)
+            return -1;
         return ul;
+    }
 
     if (device_virtdisk_matches(virtpath, "hd",
                                 &disk, 3,



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel