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

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.