Re: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source
On 07/23/2013 12:18 PM, Paolo Bonzini wrote:
Il 23/07/2013 18:01, Daniel P. Berrange ha scritto:
> On Tue, Jul 23, 2013 at 05:35:57PM +0200, Paolo Bonzini wrote:
>> Il 23/07/2013 16:14, Daniel P. Berrange ha scritto:
>>>>> Perhaps the default could be specified in a configuration file (and
the
>>>>> default should be the safe one).
>>> No, that is even worse because now the default is not predictable..
>>>
>>> We simply default to host mode and if applications want to use the
>>> other mode they can configure the XML as desired.
>>
>> Can we just forbid mode='default' for iSCSI and force the user to
>> specify host vs. direct?
>
> That would mean that apps cannot simply configure a guest volume
> without first checking to find out what type of pool it is, and
> then specifying this extra arg for iSCSI. IMHO the value of the
> <volume> XML is that you don't have to know anything about the
> pool to be able to configure it - we're completely decoupled.
Thinking more about it, it would only be needed for <disk type='volume'
device='lun'>. And for that case, some knowledge of the pool is
necessary anyway (for one thing, it won't work with filesystem or LVM
pools). So if we could forbid mode='default' for that case only, it
would be enough as far as I'm concernde.
Paolo
Using "default" in the mode field would result in the following XML
error message (I just quickly changed a test to prove the point):
121) QEMU XML-2-XML disk-source-pool-mode
... libvirt: Domain Config error : XML error: unknown source mode
'default' for volume type disk
FAILED
The XML parsing code only looks for "mode='direct'" or
"mode='host'". If
"mode" isn't present in the XML, that's when that default comes into
play. Since 'mode' is new there could be configurations where its not in
an XML file, thus a 0 (zero e.g. "default") value is provided for the field.
Once the XML is parsed we still needed a default when it's going to be
added, thus the "magic" to set the default to HOST is in qemu_conf.c in
qemuTranslateDiskSourcePool():
if (pooldef->type == VIR_STORAGE_POOL_ISCSI) {
/* Default to use the LUN's path on host */
if (!def->srcpool->mode)
def->srcpool->mode = VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST;
I think this answers your primary concern - correct?
John