Re: [libvirt] [PATCHv3 08/26] storage: Move readonly and shared flags to disk source from disk def

On 06/25/2014 10:54 AM, Peter Krempa wrote: My immediate reaction is that all backing files are generally readonly, so when would we ever label them differently? Then again, we temporarily mark files readwrite during commit. For shared, this move makes total sense. (Shared is a host-only concept - the file is read-only but must not be relabeled by libvirt because it may be shared by other domains). And for how we are using readonly (label the host image differently than if it were read-write), it also seems to make sense. If we implemented reference-counted storage source objects, the difference between shared and readonly is whether a second reference can be obtained on a file already in use. One thing is sitting a little uneasy on my mind - do we have (or need, or want) a way to affect guest ABI by the readonly designation? That is, does it ever make sense to advertise to the guest that a disk is readonly (maybe if presenting the guest a virtual DVD drive, the guest will act differently if it is emulated as a DVD-ROM vs. if it is emulated as a DVD-RW that can be burned)? And if so, I could see a case where we might want an image to be marked readonly to the guest perspective, regardless of whether the host files are labeled for readonly use. But I've spend some time thinking about it, and can't come up with any cases where having a readonly disk (guest point of view) would still require a readwrite image from the host; and that tracking whether the guest disk is readonly by deferring to whether the host source is readonly seems to be reliable. I also don't know if we will ever want to update our <backingChain> live xml to expose whether backing chain elements are temporarily using a read-write label, even though they default to readonly; or even letting the user choose between <readonly/> vs. <shared/> for backing chain elements. This patch opens up some possibilities to think about for future changes. Okay, for all my ramblings above, I still can't articulate a firm reason why this might be a bad idea, so I can live with it going in. Note that transient remains a per-guest disk item, not a per-host image item. You know, I think this ABI check is overly strict - a guest can't tell the difference between whether a host image is <shared/> or <readonly/> (the only difference between those two exclusive flags is whether other domains may use the file at the same time). But if we relax it, it should be a separate patch. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org