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