[libvirt] [PATCH] qemu: Restore the original states of PCI device when restarting daemon

[Osier Yang]

于 2011年10月29日 07:00, Eric Blake 写道:
> On 10/20/2011 04:45 AM, Osier Yang wrote:
>> This patch is to solve the problem by introducing internal XML
>> (won't be dumped to user, only dumped to status XML). The XML is:
>> <origstates>
>> <unbind/>
>> <remove_slot/>
>> <reprobe/>
>> </origstates>

Oh, I forgot to clarify this, will add when pushing.

<hostdev>
<source>
<origstates>...</origstates>
</source>
</hostdev>

only for PCI device.

>
> Which element will this be added under?
>
>> +++ b/src/conf/domain_conf.c
>> @@ -59,8 +59,12 @@ verify(VIR_DOMAIN_VIRT_LAST<= 32);
>> /* Private flags used internally by virDomainSaveStatus and
>> * virDomainLoadStatus. */
>> typedef enum {
>> - VIR_DOMAIN_XML_INTERNAL_STATUS = (1<<16), /* dump internal domain 
>> status information */
>> - VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET = (1<<17), /* dump/parse<actual> 
>> element */
>> + /* dump internal domain status information */
>> + VIR_DOMAIN_XML_INTERNAL_STATUS = (1<<16),
>> + /* dump/parse<actual> element */
>> + VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET = (1<<17),
>> + /* dump/parse original states of host PCI device */
>> + VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES = (1<<18),
>
> Is this worth a new bit, or does it fit under the already-existing 
> umbrella of VIR_DOMAIN_XML_INTERNAL_STATUS, since we are treating 
> these three bits as internal status? But we can change that later, if 
> we need.

I like a new *_PCI_ORIG_STATES more, as it allows us control the
the status XML more precisely, e.g. one only wants to dumpxml
PCI_ORIG_STATES, though there is no such requirement currently.

>
>> +++ b/src/conf/domain_conf.h
>> @@ -153,6 +153,31 @@ struct _virDomainDeviceInfo {
>> } master;
>> };
>>
>> +typedef struct _virDomainHostdevOrigStates virDomainHostdevOrigStates;
>> +typedef virDomainHostdevOrigStates *virDomainHostdevOrigStatesPtr;
>> +struct _virDomainHostdevOrigStates {
>> + union {
>
> Don't you need some sort of discrimination field outside of the union 
> which tells you which part of the union is valid? Typically we use a 
> discriminator such as 'int type; /* enum vir... */'. But until we have 
> something to distinguish from, I guess this works.

I considered this when making the patch, there is a "type" in
virDomainHostdevDef, and virDomainHostdevOrigstates won't
be used standalone. We only need it when doing something
on virDomainHostdevDef. So I think it's not necessary.

>
>> + struct {
>> + /* Does the device need to ubind from stub when
>
> s/ubind/unbind/
>
> ACK with spelling nit fixed.
>

Thanks. I pushed with the spelling fixed, and commit message
updated.