From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jim Fehlig <jfehlig@suse.com>
Subject: Re: [PATCH 3/3] libxl: libxl__device_from_disk should
 retrieve backend from xenstore
Date: Wed, 11 Feb 2015 10:18:18 -0700
Message-ID: <54DB8EDA.7010603@suse.com>
References: <1423488068-31268-1-git-send-email-wei.liu2@citrix.com>
	<1423488068-31268-4-git-send-email-wei.liu2@citrix.com>
	<21721.58650.576986.983371@mariner.uk.xensource.com>
	<20150210114950.GF27856@zion.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <20150210114950.GF27856@zion.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>, Ian Campbell <ian.campbell@citrix.com>, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

Wei Liu wrote:
> On Tue, Feb 10, 2015 at 11:01:46AM +0000, Ian Jackson wrote:
>   
>> Wei Liu writes ("[PATCH 3/3] libxl: libxl__device_from_disk should retrieve backend from xenstore"):
>>     
>>> ... if backend is not set by caller.
>>>       
>> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>>
>> as far as it goes, but I think you may want a more radical change -
>> see below.
>>
>>     
>>> Also change the function to use "goto" idiom while I was there.
>>>       
>> (Although usually it would be better to split this kind of thing into
>> a pre-patch, in this case it's small and easily reviewed.)
>>
>> Is the backend type the only missing or potentially-wrong
>> information ?  ISTM that perhaps the caller might not know the target,
>> either.
>>
>> What should happen if the caller specifies a different target in disk
>> to the one the device is actually using ?  The documentation should
>> specify which of the fields are important.
>>
>>     
>
> I'm not sure because it's not documented.
>
> We should take a step back to define the important fields first.
>
>   
>> Maybe libxl_device_disk_remove needs to call libxl_vdev_to_device_disk
>> and check that the supplied disk struct is plausible somehow.  In that
>> case it might be nice for the caller to be able to fill in only the
>> vdev.
>>
>>     
>
> If so we need to make clear in the documentation. I'm of course fine
> with this behaviour.
>
> Jim, does libvirt (as an example of libxl user) actually cares
> specifying every fields in that struct? The other user (xl) doesn't seem
> to care that much.
>   

At minimum, libvirt will populate the pdev_path, vdev, backend, and
format fields. If backend and format (which, in libvirt-speack
correspond to the 'name' and 'type' attributes on the optional <driver>
element) are not specified, they are set to LIBXL_DISK_BACKEND_UNKNOWN
and LIBXL_DISK_FORMAT_RAW respectively.

Regards,
Jim