[libvirt] [PATCH 10/15] qemu: Error out when domain starting if the cdbfilter setting conflicts

[Osier Yang]

On 2012年12月07日 09:00, Eric Blake wrote:
> On 12/05/2012 04:14 AM, Osier Yang wrote:
>> On 2012年12月05日 19:03, Jiri Denemark wrote:
>>> On Wed, Dec 05, 2012 at 18:54:42 +0800, Osier Yang wrote:
>>> ...
>>>>> However, the mainly reason I choosed to use a sub-list of domain names
>>>>> is for future extenstion, I.E. Assuming there are other disk setting
>>>>> (you never known how many they will be), we have to guarantee they are
>>>>> same among guests in future. Looking up the disk def with domain and
>>>>> disk path gives us much flexibility IMHO.
>>>>>
>>>>
>>>> So the point of the argument is: the trade between the flexibility and
>>>> the uncomfortable locks.
>>>
>>> OK, I guess we can store more info in the sharedDisks list (either
>>> today or
>>> later when we need it), we may even store the domain list there, but
>>> we don't
>>> definitely want to go through all the domains to get the required
>>> details.
>>>
>>
>> I think it should be just a few guests share the disk in practice,
>> which means in practice the sharedDisks->disks[i]->ndomains should
>> be short. And that's the other reason why I can live with the
>> locks. Though in theory, it can be all domains (assuming it's large
>> number) share the disk.
>>
>> The other question is: Aren't we already go through all domain objects
>> in many places?
>
> I'm stepping in a bit late, and haven't fully looked at the series yet,
> but one of the things that I would love for libvirt to someday have is a
> way to get a list of all virStorageVolPtr associated with any given
> virDomainPtr, and conversely, given a virStorageVolPtr, return a list of
> all virDomainPtr that use the file (whether directly, or whether as a
> backing file).

This is actually in my right next TODO list, I.E to associate the
domain and storage. See (I'm just having rest with this SG_IO set
at the half of persistent NPIV support):

https://www.redhat.com/archives/libvir-list/2012-November/msg00878.html


  To get to that point, I envision having a way for every
> <disk>  element to be tied to a virStorageVolPtr, even if it means the
> implicit creation of a transient virStoragePoolPtr directory-based pool
> for any<disk>  specified without an explicit pool.  Then, creation of a
> new domain, as well as hot-plugging of any disks to an existing domain,
> will update each affected virStorageVolPtr; and you really _should_ be
> maintaining a list of all associated domains with the disk object.
> Thus, I think searching whether a shared disk is allowed should be a
> matter of asking that shared disk what domains it is already associated
> with, rather than asking each domain whether it uses the shared disk.

I think this gets rid of the argument completely. We can introduce
a new struct as member of virStorageVolDef to store the disk settings
which have to be consistent among the domains share it. E.g.

typedef struct virStorageVolSharedSettings virStorageVolSharedSettings;
struct virStorageVolSharedSettings {
     int cdbfilter;

     /* Others in future */
};

struct _virStorageVolDef {
     char *name;
     char *key;
     int type; /* virStorageVolType enum */

     ...

     virStorageVolSharedSettings sharedSettings;

     ...
};

As each domain disk is mapped to a volume object, we can just initialize
the sharedSettings during parsing or starting. The sharedSetting can
be further used to do checking when starting, attaching.

 From the whole project p.o.v, this sounds quite nice to me.

Regards,
Osier