10:16 a.m.
On 03/14/2014 10:40 AM, Boris Fiuczynski wrote:
On 03/14/2014 01:56 PM, John Ferlan wrote:
> From: Xu Wang <gesaint(a)linux.vnet.ibm.com>
>
> Support being able to convert the Controller RASD into a virtual device
> Signed-off-by: Xu Wang <gesaint(a)linux.vnet.ibm.com>
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/Virt_VirtualSystemManagementService.c | 76 +++++++++++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
> diff --git a/src/Virt_VirtualSystemManagementService.c
b/src/Virt_VirtualSystemManagementService.c
> index 3e7785e..26de59d 100644
> --- a/src/Virt_VirtualSystemManagementService.c
> +++ b/src/Virt_VirtualSystemManagementService.c
> @@ -1838,6 +1838,53 @@ static const char *input_rasd_to_vdev(CMPIInstance *inst,
> return NULL;
> }
>
> +static const char *controller_rasd_to_vdev(CMPIInstance *inst,
> + struct virt_device *dev)
> +{
> + const char *val = NULL;
> + const char *msg = NULL;
> + int ret;
> +
> + if (cu_get_str_prop(inst, "ResourceSubType", &val) !=
CMPI_RC_OK) {
> + msg = "ControllerRASD ResourceSubType field not valid";
> + goto out;
> + }
> + dev->dev.controller.type = strdup(val);
> +
> + /* Required fields */
> + if (cu_get_u64_prop(inst, "Index",
> + &dev->dev.controller.index) != CMPI_RC_OK) {
> + msg = "ControllerRASD Index field not valid";
> + goto out;
> + }
As I wrote elsewhere this is going to prevent the libvirt-cim user from
using the libvirt auto assignment for the index.
OK - I'll rework this..
> +
> + /* Formulate our instance id from controller, controller type,
> + * and index value. This should be unique enough.
> + */
...
> @@ -3029,6 +3102,7 @@ static CMPIStatus resource_del(struct domain *dominfo,
> if (STREQ(dev->id, devid)) {
> if ((type == CIM_RES_TYPE_GRAPHICS) ||
> (type == CIM_RES_TYPE_CONSOLE) ||
> + (type == CIM_RES_TYPE_CONTROLLER) ||
What is the reason for adding the controller here?
That's original code that I haven't touched, thought about, or I guess
knew what to do with... I guess I just hadn't got this far yet. I'll
work through the remaining comments...
Xu if you have the time to provide some feedback it would be most welcome...
John
> (type == CIM_RES_TYPE_INPUT))
> cu_statusf(_BROKER, &s, CMPI_RC_OK,
"");
> else {
> @@ -3111,6 +3185,7 @@ static CMPIStatus resource_add(struct domain *dominfo,
>
Why is the controller instance not added to a running domain?
Instead of restricting it in the if statement below all I think should
be done is this (before the if in the patch below.)
if (type == CIM_RES_TYPE_CONTROLLER &&
dev != NULL && dev->id == NULL) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_FAILED,
"Add resource failed: Index property is
required.");
goto out;
}
> if ((type == CIM_RES_TYPE_GRAPHICS) ||
> (type == CIM_RES_TYPE_INPUT) ||
> + (type == CIM_RES_TYPE_CONTROLLER) ||
> (type == CIM_RES_TYPE_CONSOLE)) {
> (*count)++;
> cu_statusf(_BROKER, &s, CMPI_RC_OK, "");
> @@ -3188,6 +3263,7 @@ static CMPIStatus resource_mod(struct domain *dominfo,
>
> if ((type == CIM_RES_TYPE_GRAPHICS) ||
> (type == CIM_RES_TYPE_INPUT) ||
> + (type == CIM_RES_TYPE_CONTROLLER) ||
What is the reason for adding the controller here?
> (type == CIM_RES_TYPE_CONSOLE))
> cu_statusf(_BROKER, &s, CMPI_RC_OK,
"");
> else {
>