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@xxxxxxxxxx>
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.
*/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list