Re: [libvirt] [PATCH v3 4/7] qemu: Translate the iscsi pool/volume disk source

On 24/07/13 01:11, Paolo Bonzini wrote: >Il 23/07/2013 18:47, John Ferlan ha scritto: >>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. >>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 >Sorry, by mode='default' I really meant "no mode at all" (I was under >the false impression that you could also specify a mode='default' with >the same effect as no mode at all). > >>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? >No, my concern is that mode='host' is a bad default for the device='lun' >case. > after going through the thread...... i think the point is the relationship between the "host" and "direct" mode. if users used the "host" mode in the past, but most of them suffered from the persistent reservatioins, and "direct" mode is the answer for them, then i indent to agree with paolo that defaulting to "direct" is better. if they used the "host" mode broadly, but persistent reservation is just requirement of a small part of the users. then i don't see any problem to default to "host" mode. i.e. the point is if "direct" mode is the answer for "host" mode. but anyway, we provide good enough documentations, so if one wants to use "direct" mode, he can still change the xml to use it, any big problems here? though defaulting to "direct" is more convenient for these users.