WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Marek Marczykowski <marmarek@xxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 2 of 6 RESENT] libxl: Accept disk name in libxl_devid_to_device_disk
From: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
Date: Mon, 6 Jun 2011 12:24:08 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 06 Jun 2011 04:24:56 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <5231fa19f3e39091ff29.1307292632@devel14>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: Citrix Systems, Inc.
References: <patchbomb.1307292630@devel14> <5231fa19f3e39091ff29.1307292632@devel14>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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

<Prev in Thread] Current Thread [Next in Thread>