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

Re: [libvirt] [PATCH 5/6] conf: give each hostdevdef a parent pointer



On 02/20/2012 10:10 AM, Laine Stump wrote:
> The parent can be any type of device. It defaults to type=none, and a
> NULL pointer. The intent is that if a hostdevdef is contained in the
> def for a higher level device (e.g. virDomainNetDef), hostdev->parent
> will point to the higher level device, and type will be set to that
> type of device. This way, during attach and detach of the device,
> parent can be checked, and appropriate callouts made to do higher
> level device initialization (e.g. setting MAC address).
> 
> Also, although these hostdevs with parents will be added to a domain's
> hostdevs list, they will be treated slightly differently when
> traversing the list, e.g. virDomainHostdefDefFree for a hostdev that
> has a parent doesn't need to be called (and will be a NOP); it will
> simply be removed from the list (since the parent device object is in
> its own type-specific list, and will be freed from there).
> ---
>  src/conf/domain_conf.c |    6 ++++++
>  src/conf/domain_conf.h |    1 +
>  2 files changed, 7 insertions(+), 0 deletions(-)

Slick.  I can see where this is going - we can easily traverse all
passthrough devices, and we can also use the backpointer to easily tell
which passthrough devices are associated with a higher-level device that
libvirt actually knows more about than just an opaque host device.

> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 004cba7..12d48fb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1318,6 +1318,12 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def)
>      if (!def)
>          return;
>  
> +    /* if there is a parent device object, it will handle freeing the
> +     * memory (including the info).
> +     */
> +    if (def->parent.type != VIR_DOMAIN_DEVICE_NONE)
> +       return;
> +
>      virDomainDeviceInfoFree(def->info);
>      VIR_FREE(def);
>  }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 25019e7..924c7ec 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -344,6 +344,7 @@ enum virDomainHostdevSubsysType {
>  typedef struct _virDomainHostdevDef virDomainHostdevDef;
>  typedef virDomainHostdevDef *virDomainHostdevDefPtr;
>  struct _virDomainHostdevDef {
> +    virDomainDeviceDef parent; /* higher level Def containing this */

However, that makes virDomainHostdevDef a rather large struct, since it
is including an entire virDomainDeviceDef even if it is not otherwise
used.  Is it any better to keep the back pointer as a pointer, along the
lines of:

if (!def->parent) return;

struct _virDomainHostdefDef {
    virDomainDeviceDefPtr parent;

I guess I'll see more when I review patch 6, so no decision on ack yet.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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