[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/9] Restrict virDomain{Attach, Detach}Device to active domains



Daniel P. Berrange wrote:
> On Thu, Jan 14, 2010 at 10:42:38AM -0700, Jim Fehlig wrote:
>   
>> virDomain{Attach,Detach}Device is now only permitted on active
>> domains.  Explicitly state this restriction in the API
>> documentation and enforce it in libvirt frontend.
>> ---
>>  src/libvirt.c |   14 ++++++++++++--
>>  1 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libvirt.c b/src/libvirt.c
>> index d973907..1145561 100644
>> --- a/src/libvirt.c
>> +++ b/src/libvirt.c
>> @@ -5121,7 +5121,8 @@ error:
>>   * @domain: pointer to domain object
>>   * @xml: pointer to XML description of one device
>>   *
>> - * Create a virtual device attachment to backend.
>> + * Create a virtual device attachment to backend.  This function,
>> + * having hotplug semantics, is only allowed on an active domain.
>>   *
>>   * Returns 0 in case of success, -1 in case of failure.
>>   */
>> @@ -5142,6 +5143,10 @@ virDomainAttachDevice(virDomainPtr domain, const char *xml)
>>          virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
>>          goto error;
>>      }
>> +    if (!virDomainIsActive(domain)) {
>> +        virLibDomainError(domain, VIR_ERR_OPERATION_INVALID, __FUNCTION__);
>> +        goto error;
>> +    }
>>      conn = domain->conn;
>>  
>>      if (conn->driver->domainAttachDevice) {
>> @@ -5164,7 +5169,8 @@ error:
>>   * @domain: pointer to domain object
>>   * @xml: pointer to XML description of one device
>>   *
>> - * Destroy a virtual device attachment to backend.
>> + * Destroy a virtual device attachment to backend.  This function,
>> + * having hot-unplug semantics, is only allowed on an active domain.
>>   *
>>   * Returns 0 in case of success, -1 in case of failure.
>>   */
>> @@ -5185,6 +5191,10 @@ virDomainDetachDevice(virDomainPtr domain, const char *xml)
>>          virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
>>          goto error;
>>      }
>> +    if (!virDomainIsActive(domain)) {
>> +        virLibDomainError(domain, VIR_ERR_OPERATION_INVALID, __FUNCTION__);
>> +        goto error;
>> +    }
>>      conn = domain->conn;
>>  
>>      if (conn->driver->domainDetachDevice) {
>> -- 
>>     
>
> Agreed with the added doc comments, but I think I prefer that we do the 
> check for active in the drivers themselves, at time of use. Doing the 
> check at the top level is open to race conditions, since no locks are
> held on the objects in question at this point. Drivers will thus have
> to do extra checks themselves, making this top level one redundant.
> So I think we should just add the docs here.
>   

Ok.  Modified patch below.

Thanks,
Jim
commit d8ec244c6513b7c44956a547e56c228a4c38fbbe
Author: Jim Fehlig <jfehlig novell com>
Date:   Wed Jan 13 18:24:51 2010 -0700

    doc: restrict virDomain{Attach,Detach}Device to active domains
    
    virDomain{Attach,Detach}Device is now only permitted on active
    domains.  Explicitly state this restriction in the API
    documentation.
    
    V2: Only change doc, dropping the hunk that forced the restriction
        in libvirt frontend.

diff --git a/src/libvirt.c b/src/libvirt.c
index d973907..188b991 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5121,7 +5121,8 @@ error:
  * @domain: pointer to domain object
  * @xml: pointer to XML description of one device
  *
- * Create a virtual device attachment to backend.
+ * Create a virtual device attachment to backend.  This function,
+ * having hotplug semantics, is only allowed on an active domain.
  *
  * Returns 0 in case of success, -1 in case of failure.
  */
@@ -5164,7 +5165,8 @@ error:
  * @domain: pointer to domain object
  * @xml: pointer to XML description of one device
  *
- * Destroy a virtual device attachment to backend.
+ * Destroy a virtual device attachment to backend.  This function,
+ * having hot-unplug semantics, is only allowed on an active domain.
  *
  * Returns 0 in case of success, -1 in case of failure.
  */

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]