From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olaf Hering Subject: Re: [PATCH 3/5] libxl: add support for vscsi Date: Wed, 27 Jan 2016 11:00:12 +0100 Message-ID: <20160127100012.GA6631@gmail.com> References: <1447415999-22003-1-git-send-email-olaf@aepfle.de> <1447415999-22003-4-git-send-email-olaf@aepfle.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1447415999-22003-4-git-send-email-olaf@aepfle.de> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Ian Jackson , Wei Liu Cc: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Fri, Nov 13, Olaf Hering wrote: > Port pvscsi support from xend to libxl: How should code which converts devices from xenstore to json handle devices which got marked as "to be removed"? In my pvscsi code I set XenbusStateClosing in the vscsidev, then XenbusStateReconfiguring in vscsictrl. This causes the backend/frontend to reevaluate the vscsi bus. During that timeframe some other code can walk xenstore and find such a device. In libxl__vscsi_fill_host below such device gets the ->remove flag. This flag is handled elsewhere to indicate that its not an active device anymore. IanC asked to remove the flag and I'm converting the code to work without such flag. The question still stands what to do with such devices when filling the list of vscsidevs on a vscsictrl. Is it up to the caller to inspect the state of each vscsidev to decide what to do with it? Or should the function below just skip the device right away? I think in the current state the device would endup in d_config and in json. Olaf > +static void libxl__vscsi_fill_host(libxl__gc *gc, > + const char *devs_path, > + char **dev_dirs, > + unsigned int ndev_dirs, > + libxl_device_vscsi *v_hst) > +{ > + libxl_vscsi_dev *v_dev; > + bool parsed_ok; > + char *c, *p, *v, *s, *dev; > + unsigned int vscsi_dev_id; > + int i, r; > + > + v_hst->vscsi_devs = libxl__malloc(NOGC, ndev_dirs * sizeof(*v_dev)); > + v_hst->num_vscsi_devs = ndev_dirs; > + /* Fill each device connected to the host */ > + for (i = 0; i < ndev_dirs; i++, dev_dirs++) { > + v_dev = v_hst->vscsi_devs + i; > + libxl_vscsi_dev_init(v_dev); > + parsed_ok = false; > + r = sscanf(*dev_dirs, "dev-%u", &vscsi_dev_id); > + if (r != 1) { > + LOG(ERROR, "expected dev-N, got '%s'", *dev_dirs); > + continue; > + } > + > + dev = GCSPRINTF("%s/%s", devs_path, *dev_dirs); > + c = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/p-devname", dev)); > + p = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/p-dev", dev)); > + v = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/v-dev", dev)); > + s = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/state", dev)); > + LOG(DEBUG, "%s/state is %s", dev, s); > + if (!(c && p && v && s)) { > + LOG(ERROR, "p-devname '%s' p-dev '%s' v-dev '%s'", c, p, v); > + continue; > + } > + > + parsed_ok = vscsi_parse_pdev(gc, v_dev, c, p, v); > + switch (atoi(s)) { > + case XenbusStateUnknown: > + case XenbusStateInitialising: > + case XenbusStateInitWait: > + case XenbusStateInitialised: > + case XenbusStateConnected: > + case XenbusStateReconfiguring: > + case XenbusStateReconfigured: > + break; > + case XenbusStateClosing: > + case XenbusStateClosed: > + v_dev->remove = true; > + break; > + }