[PATCH 0 of 3] Add support for booting on multiple devices and for boot order
# HG changeset patch # User Richard Maciel <rmaciel@linux.vnet.ibm.com> # Date 1243891178 10800 # Node ID abc90cae6c08598d0ada357a6cf2f4bb2b2cd1c7 # Parent aa8e071730d2ce20064f1c0295a8005e31ef2cea Added support to convert data in the internal format to the format used by the BootDevices property Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com> diff -r aa8e071730d2 -r abc90cae6c08 schema/VSSD.mof --- a/schema/VSSD.mof Wed May 20 10:41:46 2009 -0700 +++ b/schema/VSSD.mof Mon Jun 01 18:19:38 2009 -0300 @@ -27,7 +27,7 @@ [Description ("The device to boot from when in fully-virtualized mode." "One of hd,fd,cdrom.")] - string BootDevice; + string BootDevices[]; [Description ("The emulator the guest should use during runtime.")] string Emulator; @@ -42,8 +42,8 @@ class KVM_VirtualSystemSettingData : Virt_VirtualSystemSettingData { - [Description ("The device to boot from. One of hd,fd,cdrom.")] - string BootDevice; + [Description ("The list of devices to boot from. hd,fd,cdrom.")] + string BootDevices[]; [Description ("The emulator the guest should use during runtime.")] string Emulator; diff -r aa8e071730d2 -r abc90cae6c08 src/Virt_VSSD.c --- a/src/Virt_VSSD.c Wed May 20 10:41:46 2009 -0700 +++ b/src/Virt_VSSD.c Mon Jun 01 18:19:38 2009 -0300 @@ -38,20 +38,79 @@ const static CMPIBroker *_BROKER; -static void _set_fv_prop(struct domain *dominfo, - CMPIInstance *inst) +static CMPIStatus _set_fv_prop(struct domain *dominfo, + CMPIInstance *inst) { bool fv = true; + CMPIArray *array; + CMPICount bl_ct; + CMPIStatus s = {CMPI_RC_OK, NULL}; if (dominfo->type == DOMAIN_XENFV) CMSetProperty(inst, "IsFullVirt", (CMPIValue *)&fv, CMPI_boolean); - if (dominfo->os_info.fv.boot != NULL) - CMSetProperty(inst, - "BootDevice", - (CMPIValue *)dominfo->os_info.fv.boot, - CMPI_chars); + bl_ct = dominfo->os_info.fv.bootlist_ct; + if (bl_ct > 0) { + CMPICount i; + CMPIStatus s; + + CU_DEBUG("bootlist_ct = %d", bl_ct); + + array = CMNewArray(_BROKER, + bl_ct, + CMPI_string, + &s); + + if (s.rc != CMPI_RC_OK) { + CU_DEBUG("Error creating BootDevice list"); + goto error; + } + + for (i = 0; i < bl_ct; i++) { + CMPIString *cm_str; + + CU_DEBUG("BootList[%u]=%s", + i, + dominfo->os_info.fv.bootlist[i]); + + cm_str = CMNewString(_BROKER, + (const char *)dominfo->os_info. + fv.bootlist[i], + &s); + if (s.rc != CMPI_RC_OK) { + CU_DEBUG("Error creating CMPIString"); + goto error; + } + + s = CMSetArrayElementAt(array, + i, + (CMPIValue *)&cm_str, + CMPI_string); + if (s.rc != CMPI_RC_OK) { + CU_DEBUG("Error setting BootDevice array " + "element"); + goto error; + } + } + + s = CMSetProperty(inst, + "BootDevices", + (CMPIValue *)&array, + CMPI_stringA); + + if (s.rc != CMPI_RC_OK) { + CU_DEBUG("Error setting BootDevices property"); + goto error; + } + } + return s; + + error: + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to set BootDevices property"); + return s; } static void _set_pv_prop(struct domain *dominfo, @@ -104,6 +163,7 @@ char *pfx = NULL; char *vsid = NULL; int ret = 1; + CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIObjectPath *op; struct domain *dominfo = NULL; @@ -158,7 +218,12 @@ if ((dominfo->type == DOMAIN_XENFV) || (dominfo->type == DOMAIN_KVM) || (dominfo->type == DOMAIN_QEMU)) - _set_fv_prop(dominfo, inst); + s = _set_fv_prop(dominfo, inst); + if (s.rc != CMPI_RC_OK) { + ret = 0; + goto out; + } + else if (dominfo->type == DOMAIN_XENPV) _set_pv_prop(dominfo, inst); else if (dominfo->type == DOMAIN_LXC)
diff -r aa8e071730d2 -r abc90cae6c08 src/Virt_VSSD.c --- a/src/Virt_VSSD.c Wed May 20 10:41:46 2009 -0700 +++ b/src/Virt_VSSD.c Mon Jun 01 18:19:38 2009 -0300 @@ -38,20 +38,79 @@
const static CMPIBroker *_BROKER;
-static void _set_fv_prop(struct domain *dominfo, - CMPIInstance *inst) +static CMPIStatus _set_fv_prop(struct domain *dominfo, + CMPIInstance *inst) { bool fv = true; + CMPIArray *array; + CMPICount bl_ct; + CMPIStatus s = {CMPI_RC_OK, NULL};
if (dominfo->type == DOMAIN_XENFV) CMSetProperty(inst, "IsFullVirt", (CMPIValue *)&fv, CMPI_boolean);
- if (dominfo->os_info.fv.boot != NULL) - CMSetProperty(inst, - "BootDevice", - (CMPIValue *)dominfo->os_info.fv.boot, - CMPI_chars); + bl_ct = dominfo->os_info.fv.bootlist_ct; + if (bl_ct > 0) {
If you want to avoid placing everything within an if, you could do the following instead: if (bl_ct <= 0) return s; That will save you on some indention.
+ CMPICount i; + CMPIStatus s;
CMPIStatus is already defined at the beginning of the function.
+ + CU_DEBUG("bootlist_ct = %d", bl_ct); + + array = CMNewArray(_BROKER,
You'll need to modify _set_fv_prop() so that is takes a CMPIBroker as an argument. This means instance_from_dom() will also need to take a broker. The reason for this is is because of the following call chain: get_vssd_by_name() --> _get_vssd() --> instance_from_dom() --> _set_fv_prop() get_vssd_by_name() is not a static function; it is called by association providers, and those association providers need to be able to pass in their own CMPIBroker.
+ bl_ct, + CMPI_string, + &s); + + if (s.rc != CMPI_RC_OK) { + CU_DEBUG("Error creating BootDevice list");
Call cu_statusf() here - the will give you a more specific error message.
+ goto error;
Instead of error, we generally use out instead of error.
+ } + + for (i = 0; i < bl_ct; i++) { + CMPIString *cm_str; + + CU_DEBUG("BootList[%u]=%s", + i, + dominfo->os_info.fv.bootlist[i]); + + cm_str = CMNewString(_BROKER, + (const char *)dominfo->os_info. + fv.bootlist[i], + &s); + if (s.rc != CMPI_RC_OK) { + CU_DEBUG("Error creating CMPIString"); + goto error;
Call cu_statusf() here.
+ } + + s = CMSetArrayElementAt(array, + i, + (CMPIValue *)&cm_str, + CMPI_string); + if (s.rc != CMPI_RC_OK) { + CU_DEBUG("Error setting BootDevice array " + "element"); + goto error;
Call cu_statusf() here.
+ } + } + + s = CMSetProperty(inst, + "BootDevices", + (CMPIValue *)&array, + CMPI_stringA); + + if (s.rc != CMPI_RC_OK) { + CU_DEBUG("Error setting BootDevices property"); + goto error;
Call cu_statusf() here.
+ } + } + return s;
If you remove the cu_statusf() call below, then you won't need this return here.
+ + error:
Instead of using error, use out here.
+ cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to set BootDevices property");
Remove this call, and call cu_statusf() whenever a failure occurs - that will give you more specific failure messages.
+ return s; }
static void _set_pv_prop(struct domain *dominfo, @@ -104,6 +163,7 @@ char *pfx = NULL; char *vsid = NULL; int ret = 1; + CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIObjectPath *op; struct domain *dominfo = NULL;
@@ -158,7 +218,12 @@
if ((dominfo->type == DOMAIN_XENFV) || (dominfo->type == DOMAIN_KVM) || (dominfo->type == DOMAIN_QEMU)) - _set_fv_prop(dominfo, inst); + s = _set_fv_prop(dominfo, inst); + if (s.rc != CMPI_RC_OK) { + ret = 0; + goto out;
There's several points of failure in _set_fv_prop(), so I would change instance_from_dom() so that is returns a CMPIStatus instead of a int. And in _get_vssd(), you can just call something like: s = instance_from_dom(); That way, the error code and error message will already be set. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
# HG changeset patch # User Richard Maciel <rmaciel@linux.vnet.ibm.com> # Date 1243891183 10800 # Node ID f2cb4b64756a0f0eb284198cf47834c14f0d1f24 # Parent abc90cae6c08598d0ada357a6cf2f4bb2b2cd1c7 Add support to fetch data from BootDevices property to the internal data structure Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com> diff -r abc90cae6c08 -r f2cb4b64756a src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Mon Jun 01 18:19:38 2009 -0300 +++ b/src/Virt_VirtualSystemManagementService.c Mon Jun 01 18:19:43 2009 -0300 @@ -197,6 +197,69 @@ return kvm; } +static int bootord_vssd_to_domain(CMPIInstance *inst, + struct domain *domain) +{ + int ret; + CMPICount i; + CMPIArray *bootlist; + CMPIStatus s; + CMPIData boot_elem; + char **tmp_str_arr; + + for (i = 0; i < domain->os_info.fv.bootlist_ct; i++) + free(domain->os_info.fv.bootlist[i]); + + ret = cu_get_array_prop(inst, "BootDevices", &bootlist); + + if (ret == CMPI_RC_OK) { + CMPICount bl_size; + + bl_size = CMGetArrayCount(bootlist, &s); + if (s.rc != CMPI_RC_OK) { + CU_DEBUG("Invalid BootDevice array size"); + return 0; + } + + tmp_str_arr = (char **)realloc(domain->os_info.fv.bootlist, + bl_size * sizeof(char *)); + + if (tmp_str_arr == NULL) { + CU_DEBUG("Could not alloc BootDevices array"); + return 0; + } + + for (i = 0; i < bl_size; i++) { + const char *str; + + boot_elem = CMGetArrayElementAt(bootlist, + i, + NULL); + + if (CMIsNullValue(boot_elem)) { + CU_DEBUG("Null BootDevice"); + return 0; + } + + str = CMGetCharPtr(boot_elem.value.string); + + if (s.rc != CMPI_RC_OK) { + CU_DEBUG("Could not extract char pointer from " + "CMPIArray"); + return 0; + } + + tmp_str_arr[i] = strdup(str); + } + domain->os_info.fv.bootlist_ct = bl_size; + domain->os_info.fv.bootlist = tmp_str_arr; + + } else + CU_DEBUG("Failed to get BootDevices property"); + + return 1; +} + static int fv_vssd_to_domain(CMPIInstance *inst, struct domain *domain, const char *pfx) @@ -216,12 +279,9 @@ return 0; } - ret = cu_get_str_prop(inst, "BootDevice", &val); - if (ret != CMPI_RC_OK) - val = "hd"; - - free(domain->os_info.fv.boot); - domain->os_info.fv.boot = strdup(val); + ret = bootord_vssd_to_domain(inst, domain); + if (ret != 1) + return 0; ret = cu_get_str_prop(inst, "Emulator", &val); if (ret != CMPI_RC_OK)
diff -r abc90cae6c08 -r f2cb4b64756a src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Mon Jun 01 18:19:38 2009 -0300 +++ b/src/Virt_VirtualSystemManagementService.c Mon Jun 01 18:19:43 2009 -0300 @@ -197,6 +197,69 @@ return kvm; }
+static int bootord_vssd_to_domain(CMPIInstance *inst, + struct domain *domain) +{ + int ret; + CMPICount i; + CMPIArray *bootlist; + CMPIStatus s; + CMPIData boot_elem; + char **tmp_str_arr; + + for (i = 0; i < domain->os_info.fv.bootlist_ct; i++) + free(domain->os_info.fv.bootlist[i]); + + ret = cu_get_array_prop(inst, "BootDevices", &bootlist); + + if (ret == CMPI_RC_OK) {
You can save some indention here by doing something like: if (ret != CMPI_RC_OK) { CU_DEBUG("Failed to get BootDevices property"); return 1; }
+ + str = CMGetCharPtr(boot_elem.value.string); + + if (s.rc != CMPI_RC_OK) {
Instead of checking the value of s.rc, you want to check to see if str is NULL.
+ CU_DEBUG("Could not extract char pointer from " + "CMPIArray"); + return 0; + } + + tmp_str_arr[i] = strdup(str); + } + domain->os_info.fv.bootlist_ct = bl_size; + domain->os_info.fv.bootlist = tmp_str_arr; + + } else + CU_DEBUG("Failed to get BootDevices property"); + + return 1; +} + static int fv_vssd_to_domain(CMPIInstance *inst, struct domain *domain, const char *pfx)
-- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
# HG changeset patch # User Richard Maciel <rmaciel@linux.vnet.ibm.com> # Date 1243891182 10800 # Node ID ea14dbce8b57c4586db64eba5465748db6475380 # Parent f2cb4b64756a0f0eb284198cf47834c14f0d1f24 XML generation and parsing for the BootDevices property Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com> diff -r f2cb4b64756a -r ea14dbce8b57 libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Mon Jun 01 18:19:43 2009 -0300 +++ b/libxkutil/device_parsing.c Mon Jun 01 18:19:42 2009 -0300 @@ -810,6 +810,8 @@ static int parse_os(struct domain *dominfo, xmlNode *os) { xmlNode *child; + char **blist = NULL; + unsigned bl_size = 0; for (child = os->children; child != NULL; child = child->next) { if (XSTREQ(child->name, "type")) @@ -822,10 +824,23 @@ STRPROP(dominfo, os_info.pv.cmdline, child); else if (XSTREQ(child->name, "loader")) STRPROP(dominfo, os_info.fv.loader, child); - else if (XSTREQ(child->name, "boot")) - dominfo->os_info.fv.boot = get_attr_value(child, - "dev"); - else if (XSTREQ(child->name, "init")) + else if (XSTREQ(child->name, "boot")) { + char **tmp_list = NULL; + + tmp_list = (char **)realloc(blist, + (bl_size+1) * + sizeof(char *)); + if (tmp_list == NULL) { + // Nothing you can do. Just go on. + CU_DEBUG("Could not alloc space for " + "boot device"); + continue; + } + blist = tmp_list; + + blist[bl_size] = get_attr_value(child, "dev"); + bl_size++; + } else if (XSTREQ(child->name, "init")) STRPROP(dominfo, os_info.lxc.init, child); } @@ -843,6 +858,9 @@ else dominfo->type = -1; + dominfo->os_info.fv.bootlist = blist; + dominfo->os_info.fv.bootlist_ct = bl_size; + return 1; } @@ -1001,9 +1019,15 @@ free(dom->os_info.pv.cmdline); } else if ((dom->type == DOMAIN_XENFV) || (dom->type == DOMAIN_KVM) || (dom->type == DOMAIN_QEMU)) { + int i; + free(dom->os_info.fv.type); free(dom->os_info.fv.loader); - free(dom->os_info.fv.boot); + + for (i = 0; i < dom->os_info.fv.bootlist_ct; i++) { + free(dom->os_info.fv.bootlist[i]); + } + free(dom->os_info.fv.bootlist); } else if (dom->type == DOMAIN_LXC) { free(dom->os_info.lxc.type); free(dom->os_info.lxc.init); diff -r f2cb4b64756a -r ea14dbce8b57 libxkutil/device_parsing.h --- a/libxkutil/device_parsing.h Mon Jun 01 18:19:43 2009 -0300 +++ b/libxkutil/device_parsing.h Mon Jun 01 18:19:42 2009 -0300 @@ -99,7 +99,8 @@ struct fv_os_info { char *type; /* Should always be 'hvm' */ char *loader; - char *boot; + unsigned bootlist_ct; + char **bootlist; }; struct lxc_os_info { diff -r f2cb4b64756a -r ea14dbce8b57 libxkutil/xml_parse_test.c --- a/libxkutil/xml_parse_test.c Mon Jun 01 18:19:43 2009 -0300 +++ b/libxkutil/xml_parse_test.c Mon Jun 01 18:19:42 2009 -0300 @@ -28,6 +28,7 @@ static void print_os(struct domain *dom, FILE *d) { + int i; if (dom->type == DOMAIN_XENPV) { print_value(d, "Domain Type", "Xen PV"); @@ -39,13 +40,18 @@ print_value(d, "Domain Type", "Xen FV"); print_value(d, "Type", dom->os_info.fv.type); print_value(d, "Loader", dom->os_info.fv.loader); - print_value(d, "Boot", dom->os_info.fv.boot); + for (i = 0; i < dom->os_info.fv.bootlist_ct; i++) { + print_value(d, "Boot", dom->os_info.fv.bootlist[i]); + } } else if ((dom->type == DOMAIN_KVM) || (dom->type == DOMAIN_QEMU)) { print_value(d, "Domain Type", "KVM/QEMU"); print_value(d, "Type", dom->os_info.fv.type); print_value(d, "Loader", dom->os_info.fv.loader); - print_value(d, "Boot", dom->os_info.fv.boot); + + for (i = 0; i < dom->os_info.fv.bootlist_ct; i++) { + print_value(d, "Boot", dom->os_info.fv.bootlist[i]); + } } else if (dom->type == DOMAIN_LXC) { print_value(d, "Init", dom->os_info.lxc.init); } else { diff -r f2cb4b64756a -r ea14dbce8b57 libxkutil/xmlgen.c --- a/libxkutil/xmlgen.c Mon Jun 01 18:19:43 2009 -0300 +++ b/libxkutil/xmlgen.c Mon Jun 01 18:19:42 2009 -0300 @@ -439,6 +439,7 @@ { struct fv_os_info *os = &domain->os_info.fv; xmlNodePtr tmp; + unsigned i; if (os->type == NULL) os->type = strdup("hvm"); @@ -446,8 +447,11 @@ if (os->loader == NULL) os->loader = strdup("/usr/lib/xen/boot/hvmloader"); - if (os->boot == NULL) - os->boot = strdup("hd"); + if (os->bootlist_ct == 0) { + os->bootlist_ct = 1; + os->bootlist = (char **)calloc(1, sizeof(char *)); + os->bootlist[0] = strdup("hd"); + } tmp = xmlNewChild(root, NULL, BAD_CAST "type", BAD_CAST os->type); if (tmp == NULL) @@ -457,11 +461,13 @@ if (tmp == NULL) return XML_ERROR; - tmp = xmlNewChild(root, NULL, BAD_CAST "boot", NULL); - if (tmp == NULL) - return XML_ERROR; + for (i = 0; i < os->bootlist_ct; i++) { + tmp = xmlNewChild(root, NULL, BAD_CAST "boot", NULL); + if (tmp == NULL) + return XML_ERROR; - xmlNewProp(tmp, BAD_CAST "dev", BAD_CAST os->boot); + xmlNewProp(tmp, BAD_CAST "dev", BAD_CAST os->bootlist[i]); + } tmp = xmlNewChild(root, NULL, BAD_CAST "features", NULL); xmlNewChild(tmp, NULL, BAD_CAST "pae", NULL); @@ -475,21 +481,28 @@ { struct fv_os_info *os = &domain->os_info.fv; xmlNodePtr tmp; + unsigned i; if (os->type == NULL) os->type = strdup("hvm"); - if (os->boot == NULL) - os->boot = strdup("hd"); + if (os->bootlist_ct == 0) { + os->bootlist_ct = 1; + os->bootlist = (char **)calloc(1, sizeof(char *)); + os->bootlist[0] = strdup("hd"); + } tmp = xmlNewChild(root, NULL, BAD_CAST "type", BAD_CAST os->type); if (tmp == NULL) return XML_ERROR; - tmp = xmlNewChild(root, NULL, BAD_CAST "boot", NULL); - if (tmp == NULL) - return XML_ERROR; - xmlNewProp(tmp, BAD_CAST "dev", BAD_CAST os->boot); + for (i = 0; i < os->bootlist_ct; i++) { + tmp = xmlNewChild(root, NULL, BAD_CAST "boot", NULL); + if (tmp == NULL) + return XML_ERROR; + + xmlNewProp(tmp, BAD_CAST "dev", BAD_CAST os->bootlist[i]); + } return NULL; }
diff -r f2cb4b64756a -r ea14dbce8b57 libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c Mon Jun 01 18:19:43 2009 -0300 +++ b/libxkutil/device_parsing.c Mon Jun 01 18:19:42 2009 -0300 @@ -810,6 +810,8 @@ static int parse_os(struct domain *dominfo, xmlNode *os) { xmlNode *child; + char **blist = NULL; + unsigned bl_size = 0;
for (child = os->children; child != NULL; child = child->next) { if (XSTREQ(child->name, "type")) @@ -822,10 +824,23 @@ STRPROP(dominfo, os_info.pv.cmdline, child); else if (XSTREQ(child->name, "loader")) STRPROP(dominfo, os_info.fv.loader, child); - else if (XSTREQ(child->name, "boot")) - dominfo->os_info.fv.boot = get_attr_value(child, - "dev"); - else if (XSTREQ(child->name, "init")) + else if (XSTREQ(child->name, "boot")) { + char **tmp_list = NULL; + + tmp_list = (char **)realloc(blist, + (bl_size+1) *
This should be (bl_size + 1)
diff -r f2cb4b64756a -r ea14dbce8b57 libxkutil/xmlgen.c
@@ -475,21 +481,28 @@ { struct fv_os_info *os = &domain->os_info.fv; xmlNodePtr tmp; + unsigned i;
if (os->type == NULL) os->type = strdup("hvm");
- if (os->boot == NULL) - os->boot = strdup("hd"); + if (os->bootlist_ct == 0) { + os->bootlist_ct = 1; + os->bootlist = (char **)calloc(1, sizeof(char *)); + os->bootlist[0] = strdup("hd"); + }
tmp = xmlNewChild(root, NULL, BAD_CAST "type", BAD_CAST os->type); if (tmp == NULL) return XML_ERROR;
- tmp = xmlNewChild(root, NULL, BAD_CAST "boot", NULL); - if (tmp == NULL) - return XML_ERROR; - xmlNewProp(tmp, BAD_CAST "dev", BAD_CAST os->boot); + for (i = 0; i < os->bootlist_ct; i++) { + tmp = xmlNewChild(root, NULL, BAD_CAST "boot", NULL); + if (tmp == NULL) + return XML_ERROR; + + xmlNewProp(tmp, BAD_CAST "dev", BAD_CAST os->bootlist[i]); + }
This bit of code is identical between Xen and KVM, so I would create a helper function that modifies the root node. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (2)
-
Kaitlin Rupert -
Richard Maciel