Re: [PATCH 2/6] libxl: Remove linux udev rules

[Konrad Rzeszutek Wilk <konrad@darnok.org>]

On Tue, Jul 07, 2015 at 12:39:52PM +0100, Wei Liu wrote:
> On Mon, Jul 06, 2015 at 11:51:39AM +0100, George Dunlap wrote:
> > They are no longer needed, having been replaced by a daemon for
> > driverdomains which will run scripts as necessary.

This introduces an regression. The 'daemon for driverdomains'
is xenbackendd - which only gets built for NetBSD.

That means we can't do Linux driver domains anymore with this patch.


> > 
> > Worse yet, they seem to be broken for script-based block devices, such
> > as block-iscsi.  This wouldn't matter so much if they were never run
> > by default; but if you run block-attach without having created a
> > domain, then the appropriate node to disable running udev scripts will
> > not have been written yet, and the attach will silently fail.
> > 
> > Rather than try to sort out that issue, just remove them entirely.
> > 
> > Note: This changes tools/configure.ac, so autogen.sh may need to be
> > re-run.
> > 
> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> > ---
> > CC: Ian Campbell <ian.campbell@citrix.com>
> > CC: Ian Jackson <ian.jackson@citrix.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> > CC: Roger Pau Monne <roger.pau@citrix.com>
> > 
> > There was some concern that old udev scripts may end up lying around
> > in /etc which will still trigger and run unless libxl/disable_udev
> > exists.
> > 
> 
> It's certainly not making things worse, right?
> 
> I think it's user's responsibility to make sure the installation is
> clean. It's the same as he / she should avoid having stale libraries
> around.
> 
> > If that's a concern, then for one release we might consider writing
> > libxl/disable_udev manually in xencommons or something to make sure
> > that they don't trigger.
> > ---
> >  tools/configure                              | 13 ++++++-------
> [...]
> >      }
> >      libxl_vminfo_list_free(vm_list, nb_vm);
> > -    int hotplug_setting = libxl__hotplug_settings(gc, t);
> > -    if (hotplug_setting < 0) {
> > -        LOG(ERROR, "unable to get current hotplug scripts execution setting");
> > -        rc = ERROR_FAIL;
> > -        goto out;
> > -    }
> > -    if (libxl_defbool_val(info->run_hotplug_scripts) != hotplug_setting &&
> > -        (nb_vm - 1)) {
> > -        LOG(ERROR, "cannot change hotplug execution option once set, "
> > -                    "please shutdown all guests before changing it");
> > -        rc = ERROR_FAIL;
> > -        goto out;
> > -    }
> > -
> > -    if (libxl_defbool_val(info->run_hotplug_scripts)) {
> > -        rc = libxl__xs_write_checked(gc, t, DISABLE_UDEV_PATH, "1");
> > -        if (rc) {
> > -            LOGE(ERROR, "unable to write %s = 1", DISABLE_UDEV_PATH);
> > -            goto out;
> > -        }
> > -    } else {
> > -        rc = libxl__xs_rm_checked(gc, t, DISABLE_UDEV_PATH);
> > -        if (rc) {
> > -            LOGE(ERROR, "unable to delete %s", DISABLE_UDEV_PATH);
> > -            goto out;
> > -        }
> 
> Heh, looks like this is buggy as it is -- every guest shares the same
> node. I.e. if a guest has run_hotplug_script in xl config file it can
> overwrite that node easily.
> 
> Wei.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel