Re: [Xen-devel] [PATCH 6/8] libxl: Allow local access for block devices with hotplug scripts

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Thu, Mar 17, 2016 at 6:36 PM, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> wrote:
> George Dunlap writes ("[PATCH 6/8] libxl: Allow local access for block 
> devices with hotplug scripts"):
>> pygrub and qemuu need to be able to access a VM's disks locally in
>> order to be able to pull out the kernel and provide emulated disk
>> access, respectively.  This can be done either by accessing the local
>> disk directly, or by plugging the target disk into dom0 to allow
>> access.
>
> Some long lines, `if(', etc., in this patch, I'm afraid.
>
>> Unfortunately, while the plugging machinery works for pygrub, it does
>> not yet work for qemuu; meaning that disk hotplug scripts cannot be
>> used with HVM domains.
>>
>> Fortunately, disks using hotplug scripts created in dom0 do create a
>> block device as part of set-up, which can be accessed locally; and if
>> they use block-common.sh:write_dev, this path will bre written to
>> physical-device-path.
>>
>> Modify libxl__device_disk_setdefault() to be able to fish this path
>> out of xenstore and pass it back.
>>
>> We need the target domid to find the appropriate xenstore node, so add
>> that to libxl__disk_local_state.
>
> I couldn't find that change in this patch.

Oops -- forgot to take this paragraph out after porting it on top of
patch 2 (which changes libxl not to duplicate the work of 'block'
anymore).  I'll remove this.

>
>> +    /*
>> +     * If the format isn't raw and / or we're using a script, then see
>> +     * if the script has written a path to the "cooked" node
>> +     */
>> +    if(disk->script && domid != INVALID_DOMID) {
>> +        libxl__device device;
>> +        char *be_path, *pdpath;
>> +        int rc;
>
> I don't see where you check that the disk is not being provided by a
> driver domain - in which case the hotplug script ran in the driver
> domain and the device path is also only in the driver domain.
>
> (By `driver domain' I mean, really, a domain other than this one.)

It's in patch 4 (still at the top of this function):

+    /* No local paths for driver domains */
+    if (disk->backend_domname != NULL) {
+        LOG(DEBUG, "Non-local backend, can't access locally.\n");
+        goto out;
+    }

>
>>  _hidden char * libxl__device_disk_find_local_path(libxl__gc *gc,
>> +                                                  libxl_domid domid,
>
> This new parameter would maybe be a little less confusing if it were
> explicitly `guest_domid'; after all it might be the driver domain
> domid.

Ack.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel