[Libvirt-cim] [PATCH] (#4) Changed the output of AllocationCapability association when using DiskPool as input

Kaitlin Rupert kaitlin at linux.vnet.ibm.com
Fri Mar 27 21:36:38 UTC 2009


> +static CMPIStatus set_disk_props(int type,
> +                                 const CMPIObjectPath *ref,
> +                                 const char *id,
> +                                 const char *disk_path,
> +                                 uint64_t disk_size,
> +                                 uint16_t emu_type,
> +                                 struct inst_list *list)
> +{
> +        const char *dev;
> +        CMPIInstance *inst;
> +        CMPIStatus s = {CMPI_RC_OK, NULL};
> +
> +        if (type == DOMAIN_LXC) {
> +                dev = "/lxc_mnt/tmp";
> +        }
> +        else {
> +                dev = "hda";
> +        }
> +
> +        inst = sdc_rasd_inst(&s, ref, CIM_RES_TYPE_DISK);
> +        if ((inst == NULL) || (s.rc != CMPI_RC_OK))
> +                goto out;
> +
> +        CMSetProperty(inst, "InstanceID", (CMPIValue *)id, CMPI_chars);
> +        CMSetProperty(inst, "AllocationQuantity",
> +                      (CMPIValue *)"MegaBytes", CMPI_chars);
> +        CMSetProperty(inst, "Address", (CMPIValue *)disk_path, CMPI_chars);
> +
> +#if !VIR_USE_LIBVIRT_STORAGE
> +        CMSetProperty(inst, "VirtualQuantity",
> +                      (CMPIValue *)&disk_size, CMPI_uint64);
> +#endif

There's no need to set the VirtualQuantity for the CDROM.  I know this 
is set in the current code, but it really doesn't make much sense.

So I would remove this case.

> +
> +        if (type == DOMAIN_LXC)
> +                CMSetProperty(inst, "MountPoint", (CMPIValue *)dev, CMPI_chars);
> +        else {
> +#if VIR_USE_LIBVIRT_STORAGE
> +                if (emu_type == 0)
> +                        CMSetProperty(inst, "VirtualQuantity",
> +                                      (CMPIValue *)&disk_size, CMPI_uint64);
> +#endif

You can remove the #if / #endif here.  Just set the VirtualQuantity in 
the case where emu_type == 0.

> +
> +                if (type == DOMAIN_XENPV) {
> +                        dev = "xvda";
> +                        CMSetProperty(inst, "Caption",
> +                                      (CMPIValue *)"PV disk", CMPI_chars);
> +                } else if (type == DOMAIN_XENFV) {
> +                        CMSetProperty(inst, "Caption",
> +                                      (CMPIValue *)"FV disk", CMPI_chars);
> +                }
> +
> +                CMSetProperty(inst, "VirtualDevice",
> +                              (CMPIValue *)dev, CMPI_chars);
> +                CMSetProperty(inst, "EmulatedType",
> +                              (CMPIValue *)&emu_type, CMPI_uint16);
> +        }
> +
> +        inst_list_add(list, inst);
> +
> + out:
> +        return s;
> +}
> +
> +#if VIR_USE_LIBVIRT_STORAGE
> +static CMPIStatus cdrom_template(const CMPIObjectPath *ref,
> +                                  int template_type,
> +                                  struct inst_list *list)

Why not use the CDROM template for both the VIR_USE_LIBVIRT_STORAGE and 
the !VIR_USE_LIBVIRT_STORAGE cases?

The CDROM device should look the same regardless of whether this is 
storage pool support.  Why not call this to handle CDROM devices in both 
situations?

> +static CMPIStatus lxc_template(const CMPIObjectPath *ref,
> +                               int template_type,
> +                               struct inst_list *list)
> +{
> +        uint64_t vol_size = 0;
> +        int emu_type = 0;
> +        const char *vol_path = "/tmp";
> +        const char *id;

This function looks really similar to the disk_template() function (for 
the !VIR_USE_LIBVIRT_STORAGE case).

Why not use the same function for both?  This is why I suggested using a 
default_disk_template() function.

> +
> +        CMPIStatus s = {CMPI_RC_OK, NULL};
> +        
> +        switch(template_type) {
> +        case SDC_RASD_MIN:
> +                id = "Minimum";
> +                break;
> +        case SDC_RASD_MAX:
> +                id = "Maximum";
> +                break;
> +        case SDC_RASD_INC:
> +                id = "Increment";
> +                break;
> +        case SDC_RASD_DEF:
> +                id = "Default";
> +                break;
> +        default:
> +                cu_statusf(_BROKER, &s,
> +                           CMPI_RC_ERR_FAILED,
> +                           "Unsupported sdc_rasd type");
> +                goto out;
> +        }
> +
> +        s = set_disk_props(DOMAIN_LXC,
> +                           ref, 
> +                           id, 
> +                           vol_path,
> +                           vol_size, 
> +                           emu_type, 
> +                           list); 
> +
> + out:
> +        return s;
> +
> +}

>  static CMPIStatus disk_template(const CMPIObjectPath *ref,
>                                  int template_type,
>                                  struct inst_list *list)

This code looks similar to the lxc_template() code.  Why not move this 
code to a separate function?

> @@ -673,6 +1006,7 @@
>          const char *id;
>          uint64_t disk_size;
>          uint16_t emu_type = 0;
> +        const char *addr = "/dev/null";
>          CMPIStatus s = {CMPI_RC_OK, NULL};
> 
>          switch(template_type) {
> @@ -702,7 +1036,6 @@
>          }
> 
>          pfx = class_prefix_name(CLASSNAME(ref));
> -
>          if (STREQ(pfx, "Xen")) {
>                  int xen_type[2] = {DOMAIN_XENFV, DOMAIN_XENPV};
>                  int i = 0;
> @@ -712,6 +1045,7 @@
>                          s = set_disk_props(xen_type[i],
>                                             ref, 
>                                             id, 
> +                                           addr,
>                                             disk_size, 
>                                             emu_type, 
>                                             list); 
> @@ -721,7 +1055,8 @@
>                          emu_type = 1;
>                          s = set_disk_props(xen_type[i],
>                                             ref, 
> -                                           id, 
> +                                           id,
> +                                           addr, 
>                                             disk_size, 
>                                             emu_type, 
>                                             list); 
> @@ -731,7 +1066,8 @@
>          } else if (STREQ(pfx, "KVM")) {
>                  s = set_disk_props(DOMAIN_KVM,
>                                     ref, 
> -                                   id, 
> +                                   id,
> +                                   addr, 
>                                     disk_size, 
>                                     emu_type, 
>                                     list); 
> @@ -742,13 +1078,16 @@
>                  s = set_disk_props(DOMAIN_KVM,
>                                     ref, 
>                                     id, 
> +                                   addr,
>                                     disk_size, 
>                                     emu_type, 
>                                     list); 
>          } else if (STREQ(pfx, "LXC")) {
> +                addr = "/tmp";
>                  s = set_disk_props(DOMAIN_LXC,
>                                     ref, 
>                                     id, 
> +                                   addr,
>                                     disk_size, 
>                                     emu_type, 
>                                     list); 
> @@ -761,6 +1100,7 @@
>   out:
>          return s;
>  }
> +#endif
> 
>  static CMPIStatus graphics_template(const CMPIObjectPath *ref,
>                                      int template_type,
> 
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim


-- 
Kaitlin Rupert
IBM Linux Technology Center
kaitlin at linux.vnet.ibm.com




More information about the Libvirt-cim mailing list