On Fri, 2011-09-16 at 16:36 +0100, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
> # Date 1316186985 -7200
> # Node ID 00949e363f6f2c70001da548403475628df93b97
> # Parent 63e254468d6e8d81baeafaed68f05791dc21eb4e
> hotplug: add hotplug-status disconnected
>
> Added hotplug-status disconnected when vbd are removed.
>
> Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
>
Generally looks good, but I don't think you've got the logical
separation between the two patches quite right:
> diff -r 63e254468d6e -r 00949e363f6f tools/xenbackendd/xenbackendd.c
> --- a/tools/xenbackendd/xenbackendd.c Wed Sep 14 14:18:40 2011 +0200
> +++ b/tools/xenbackendd/xenbackendd.c Fri Sep 16 17:29:45 2011 +0200
> @@ -169,7 +172,7 @@ main(int argc, char * const argv[])
> log_file = optarg;
> break;
> case 'p':
> - pidfile = pidfile;
> + pidfile = optarg;
This is an unrelated bugfix? If so please post it as such.
> case 's':
> vbd_script = optarg;
> break;
> @@ -297,11 +300,38 @@ main(int argc, char * const argv[])
> strerror(errno));
> goto next2;
> }
> - doexec(s, vec[XS_WATCH_PATH], sstate);
> + doexec(s, vec[XS_WATCH_PATH], sstate, NULL);
> break;
>
> case DEVTYPE_VBD:
> - doexec(vbd_script, vec[XS_WATCH_PATH], sstate);
> + /* check if given file is a block device or a raw image
> */
> + snprintf(buf, sizeof(buf), "%s/params",
> vec[XS_WATCH_PATH]);
> + params = xs_read(xs, XBT_NULL, buf, 0);
> + if(params == NULL) {
> + dolog(LOG_ERR,
> + "Failed to read %s (%s)", buf,
> strerror(errno));
> + goto next2;
> + }
> + if (stat(params, &stab) < 0) {
> + dolog(LOG_ERR,
> + "Failed to get info about %s (%s)", params,
> + strerror(errno));
> + goto next3;
> + }
> + stype = NULL;
> + if (S_ISBLK(stab.st_mode))
> + stype = "phy";
> + if (S_ISREG(stab.st_mode))
> + stype = "file";
> + if (stype == NULL) {
> + dolog(LOG_ERR,
> + "Failed to attach %s (not a block device or
> raw image)",
> + params, strerror(errno));
> + goto next3;
> + }
> + doexec(vbd_script, vec[XS_WATCH_PATH], sstate, stype);
> +next3:
> + free(params);
Isn't this more like part of patch 2/2? It doesn't seem to relate to the
addition of the hotplug-status xenstore node.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home