Re: [Xen-devel] [PATCH 1 of 2] hotplug: add hotplug-status disconnected

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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