Re: [PATCH v4 07/14] conf: Enable cold-plug of a mediated device

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

 



On 03/22/2017 11:27 AM, Erik Skultety wrote:
> This merely introduces virDomainHostdevMatchSubsysMediatedDev method that
> is supposed to check whether device being cold-plugged does not already
> exist in the domain configuration.
> 
> Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
> ---
>  src/conf/domain_conf.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 63ac65e8ab..a4ed605c27 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -14375,6 +14375,19 @@ virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr first,
>  }
>  
>  static int
> +virDomainHostdevMatchSubsysMediatedDev(virDomainHostdevDefPtr a,
> +                                       virDomainHostdevDefPtr b)
> +{
> +    virDomainHostdevSubsysMediatedDevPtr src_a = &a->source.subsys.u.mdev;
> +    virDomainHostdevSubsysMediatedDevPtr src_b = &b->source.subsys.u.mdev;
> +
> +    if (STREQ(src_a->uuidstr, src_b->uuidstr))
> +        return 1;
> +
> +    return 0;
> +}
> +
> +static int
>  virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
>                              virDomainHostdevDefPtr b)
>  {
> @@ -14405,6 +14418,7 @@ virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
>          else
>              return 0;
>      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> +        return virDomainHostdevMatchSubsysMediatedDev(a, b);

This points out that the practice of typecasting all values that are
being used for a switch() and removing the default: case isn't really
all that big of a help. Sure, it forces you to add new clauses to all
the relevant switches in the patch where you add the new enum value, but
it would just add in an *empty* clause thinking "I'll fill it in later",
then you're once again relegating the responsibility for adding the code
to your own memory, which is exactly what we were trying to avoid :-P

(Nothing wrong with what you're doing, I just felt like pontificating a
bit :-)


>      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
>          return 0;
>      }
> 


ACK.

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux