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