On 13/08/14 19:48 -0600, Eric Blake wrote:

While investigating active commit, I noticed that our code for probing backing chains currently guesses at type="file" vs. type="block" solely based on stat results (regular file vs. block device). Is a block device allowed to be used as <disk type='block' device='lun'> at the same time it has qcow2 format, or does use of device='lun' enforce that the block device is used with raw format? My worry is that if a lun device can be used with qcow2 format, then it can have a non-block backing file, and an active commit would convert <disk type='block' device='lun'> into <disk type='file' device='lun'> which is not valid.

On a related vein, that means that an active commit currently auto-picks whether the <disk> will be listed as type='file' or type='block' solely on the stat results. Elsewhere, we allow type='file' even for block devices, where a noticeable difference is how virDomainGetBlockInfo() reports allocation (for type='file', allocation is based on how sparse the file is, but a block device is not sparse; for type='block', allocation is based on asking qemu the maximum used cluster). Should we be auto-converting to type='block' in other cases where we have stat results? For example, I posted a patch today to let the user explicitly request that virsh blockcopy to a block device will use type='block' because the existing code has always used type='file', but if we were doing things automatically, then the user would automatically get type='block' for block device destinations instead of having to request it; but has no way to forcefully list a file even when the destination is a block (at least, not until I implement virDomainBlockCopy() that takes an XML <disk> description).

One drawback of autoconversion is the XML changes - with type='file', the host resource is located at xpath disk/source/@file, but with type='block', it is at xpath disk/source/@dev. If we ever convert a user's type='file' into a block device, but the user isn't expecting the conversion, then they won't be able to find the source file name in the output XML. So on that ground, autoprobing type='block' for active commit is fishy; we really should be honoring the user's input for <backingStore> rather than probing it ourselves every time. Until that point, and given that I just proposed a patch to turn on type='block' for blockcopy, where type='file' is used in all other cases, should we have the same sort of flag for active commit?

Would the following semantics be desirable with respect to disk type conversion? We introduce flag(s) to permit type conversion from file->block (do we need one for the other way too)? For active commit, if one of these flags is not specified, no conversion will be done. If the parent volume type is not the same as the active layer then an error will be trigerred. In other words, the user is responsible for indicating which way a conversion should go if the types are different. -- Adam Litke