From: Peter Dinda Date: Wed, 20 Jun 2012 01:48:39 +0000 (-0500) Subject: More extensive error checking in checkpoint/restore + other cleanup X-Git-Url: http://v3vee.org/palacios/gitweb/gitweb.cgi?a=commitdiff_plain;h=46255dee5894bc5e3be5f54a5866c0c4b30b3896;p=palacios.git More extensive error checking in checkpoint/restore + other cleanup --- diff --git a/palacios/src/palacios/svm.c b/palacios/src/palacios/svm.c index 4d148e9..434e3bd 100644 --- a/palacios/src/palacios/svm.c +++ b/palacios/src/palacios/svm.c @@ -368,15 +368,25 @@ int v3_deinit_svm_vmcb(struct guest_info * core) { #ifdef V3_CONFIG_CHECKPOINT int v3_svm_save_core(struct guest_info * core, void * ctx){ - v3_chkpt_save_8(ctx, "cpl", &(core->cpl)); - v3_chkpt_save(ctx, "vmcb_data", PAGE_SIZE, core->vmm_data); + if (v3_chkpt_save_8(ctx, "cpl", &(core->cpl)) == -1) { + PrintError("Could not save SVM cpl\n"); + return -1; + } + + if (v3_chkpt_save(ctx, "vmcb_data", PAGE_SIZE, core->vmm_data) == -1) { + PrintError("Could not save SVM vmcb\n"); + return -1; + } return 0; } int v3_svm_load_core(struct guest_info * core, void * ctx){ - v3_chkpt_load_8(ctx, "cpl", &(core->cpl)); + if (v3_chkpt_load_8(ctx, "cpl", &(core->cpl)) == -1) { + PrintError("Could not load SVM cpl\n"); + return -1; + } if (v3_chkpt_load(ctx, "vmcb_data", PAGE_SIZE, core->vmm_data) == -1) { return -1; diff --git a/palacios/src/palacios/vmm_checkpoint.c b/palacios/src/palacios/vmm_checkpoint.c index 5ba96ac..ef99cdf 100644 --- a/palacios/src/palacios/vmm_checkpoint.c +++ b/palacios/src/palacios/vmm_checkpoint.c @@ -161,6 +161,12 @@ struct v3_chkpt_ctx * v3_chkpt_open_ctx(struct v3_chkpt * chkpt, struct v3_chkpt struct v3_chkpt_ctx * ctx = V3_Malloc(sizeof(struct v3_chkpt_ctx)); void * parent_store_ctx = NULL; + + if (!ctx) { + PrintError("Unable to allocate context\n"); + return 0; + } + memset(ctx, 0, sizeof(struct v3_chkpt_ctx)); ctx->chkpt = chkpt; @@ -172,6 +178,10 @@ struct v3_chkpt_ctx * v3_chkpt_open_ctx(struct v3_chkpt * chkpt, struct v3_chkpt ctx->store_ctx = chkpt->interface->open_ctx(chkpt->store_data, parent_store_ctx, name); + if (!(ctx->store_ctx)) { + PrintError("Warning: opening underlying representation returned null\n"); + } + return ctx; } @@ -192,12 +202,15 @@ int v3_chkpt_close_ctx(struct v3_chkpt_ctx * ctx) { int v3_chkpt_save(struct v3_chkpt_ctx * ctx, char * tag, uint64_t len, void * buf) { struct v3_chkpt * chkpt = ctx->chkpt; - return chkpt->interface->save(chkpt->store_data, ctx->store_ctx, tag, len, buf); + + return chkpt->interface->save(chkpt->store_data, ctx->store_ctx, tag, len, buf); + } int v3_chkpt_load(struct v3_chkpt_ctx * ctx, char * tag, uint64_t len, void * buf) { struct v3_chkpt * chkpt = ctx->chkpt; + return chkpt->interface->load(chkpt->store_data, ctx->store_ctx, tag, len, buf); } @@ -212,11 +225,21 @@ static int load_memory(struct v3_vm_info * vm, struct v3_chkpt * chkpt) { guest_mem_base = V3_VAddr((void *)vm->mem_map.base_region.host_addr); ctx = v3_chkpt_open_ctx(chkpt, NULL, "memory_img"); - - ret = v3_chkpt_load(ctx, "memory_img", vm->mem_size, guest_mem_base); + + if (!ctx) { + PrintError("Unable to open context for memory load\n"); + return -1; + } + + if (v3_chkpt_load(ctx, "memory_img", vm->mem_size, guest_mem_base) == -1) { + PrintError("Unable to load all of memory (requested=%llu bytes, result=%llu bytes\n",(uint64_t)(vm->mem_size),ret); + v3_chkpt_close_ctx(ctx); + return -1; + } + v3_chkpt_close_ctx(ctx); - return ret; + return 0; } @@ -229,11 +252,20 @@ static int save_memory(struct v3_vm_info * vm, struct v3_chkpt * chkpt) { ctx = v3_chkpt_open_ctx(chkpt, NULL,"memory_img"); + if (!ctx) { + PrintError("Unable to open context to save memory\n"); + return -1; + } + + if (v3_chkpt_save(ctx, "memory_img", vm->mem_size, guest_mem_base) == -1) { + PrintError("Unable to load all of memory (requested=%llu, received=%llu)\n",(uint64_t)(vm->mem_size),ret); + v3_chkpt_close_ctx(ctx); + return -1; + } - ret = v3_chkpt_save(ctx, "memory_img", vm->mem_size, guest_mem_base); v3_chkpt_close_ctx(ctx); - return ret; + return 0; } int save_header(struct v3_vm_info * vm, struct v3_chkpt * chkpt) { @@ -241,17 +273,29 @@ int save_header(struct v3_vm_info * vm, struct v3_chkpt * chkpt) { void * ctx = NULL; ctx = v3_chkpt_open_ctx(chkpt, NULL, "header"); + if (!ctx) { + PrintError("Cannot open context to save header\n"); + return -1; + } switch (v3_mach_type) { case V3_SVM_CPU: case V3_SVM_REV3_CPU: { - v3_chkpt_save(ctx, "header", strlen(svm_chkpt_header), svm_chkpt_header); + if (v3_chkpt_save(ctx, "header", strlen(svm_chkpt_header), svm_chkpt_header) == -1) { + PrintError("Could not save all of SVM header\n"); + v3_chkpt_close_ctx(ctx); + return -1; + } break; } case V3_VMX_CPU: case V3_VMX_EPT_CPU: case V3_VMX_EPT_UG_CPU: { - v3_chkpt_save(ctx, "header", strlen(vmx_chkpt_header), vmx_chkpt_header); + if (v3_chkpt_save(ctx, "header", strlen(vmx_chkpt_header), vmx_chkpt_header) == -1) { + PrintError("Could not save all of VMX header\n"); + v3_chkpt_close_ctx(ctx); + return -1; + } break; } default: @@ -276,7 +320,13 @@ static int load_header(struct v3_vm_info * vm, struct v3_chkpt * chkpt) { case V3_SVM_REV3_CPU: { char header[strlen(svm_chkpt_header) + 1]; - v3_chkpt_load(ctx, "header", strlen(svm_chkpt_header), header); + if (v3_chkpt_load(ctx, "header", strlen(svm_chkpt_header), header) == -1) { + PrintError("Could not load all of SVM header\n"); + v3_chkpt_close_ctx(ctx); + return -1; + } + + header[strlen(svm_chkpt_header)] = 0; break; } @@ -285,7 +335,13 @@ static int load_header(struct v3_vm_info * vm, struct v3_chkpt * chkpt) { case V3_VMX_EPT_UG_CPU: { char header[strlen(vmx_chkpt_header) + 1]; - v3_chkpt_load(ctx, "header", strlen(vmx_chkpt_header), header); + if (v3_chkpt_load(ctx, "header", strlen(vmx_chkpt_header), header) == -1) { + PrintError("Could not load all of VMX header\n"); + v3_chkpt_close_ctx(ctx); + return -1; + } + + header[strlen(vmx_chkpt_header)] = 0; break; } @@ -294,9 +350,9 @@ static int load_header(struct v3_vm_info * vm, struct v3_chkpt * chkpt) { v3_chkpt_close_ctx(ctx); return -1; } - + v3_chkpt_close_ctx(ctx); - + return 0; } @@ -305,12 +361,20 @@ static int load_core(struct guest_info * info, struct v3_chkpt * chkpt) { extern v3_cpu_arch_t v3_mach_type; void * ctx = NULL; char key_name[16]; + memset(key_name, 0, 16); snprintf(key_name, 16, "guest_info%d", info->vcpu_id); ctx = v3_chkpt_open_ctx(chkpt, NULL, key_name); + if (!ctx) { + PrintError("Could not open context to load core\n"); + return -1; + } + + // These really need to have error checking + v3_chkpt_load_64(ctx, "RIP", &(info->rip)); V3_CHKPT_STD_LOAD(ctx, info->vm_regs); @@ -327,6 +391,7 @@ static int load_core(struct guest_info * info, struct v3_chkpt * chkpt) { V3_CHKPT_STD_LOAD(ctx, info->shdw_pg_state.guest_cr3); V3_CHKPT_STD_LOAD(ctx, info->shdw_pg_state.guest_cr0); V3_CHKPT_STD_LOAD(ctx, info->shdw_pg_state.guest_efer); + v3_chkpt_close_ctx(ctx); PrintDebug("Finished reading guest_info information\n"); @@ -356,9 +421,15 @@ static int load_core(struct guest_info * info, struct v3_chkpt * chkpt) { snprintf(key_name, 16, "vmcb_data%d", info->vcpu_id); ctx = v3_chkpt_open_ctx(chkpt, NULL, key_name); + + if (!ctx) { + PrintError("Could not open context to load SVM core\n"); + return -1; + } if (v3_svm_load_core(info, ctx) == -1) { PrintError("Failed to patch core %d\n", info->vcpu_id); + v3_chkpt_close_ctx(ctx); return -1; } @@ -372,10 +443,17 @@ static int load_core(struct guest_info * info, struct v3_chkpt * chkpt) { char key_name[16]; snprintf(key_name, 16, "vmcs_data%d", info->vcpu_id); + ctx = v3_chkpt_open_ctx(chkpt, NULL, key_name); + + if (!ctx) { + PrintError("Could not open context to load VMX core\n"); + return -1; + } if (v3_vmx_load_core(info, ctx) < 0) { PrintError("VMX checkpoint failed\n"); + v3_chkpt_close_ctx(ctx); return -1; } @@ -407,7 +485,14 @@ static int save_core(struct guest_info * info, struct v3_chkpt * chkpt) { snprintf(key_name, 16, "guest_info%d", info->vcpu_id); ctx = v3_chkpt_open_ctx(chkpt, NULL, key_name); + + if (!ctx) { + PrintError("Unable to open context to save core\n"); + return -1; + } + + // Error checking of all this needs to happen v3_chkpt_save_64(ctx, "RIP", &(info->rip)); V3_CHKPT_STD_SAVE(ctx, info->vm_regs); @@ -437,6 +522,11 @@ static int save_core(struct guest_info * info, struct v3_chkpt * chkpt) { snprintf(key_name, 16, "vmcb_data%d", info->vcpu_id); ctx = v3_chkpt_open_ctx(chkpt, NULL, key_name); + + if (!ctx) { + PrintError("Could not open context to store SVM core\n"); + return -1; + } if (v3_svm_save_core(info, ctx) == -1) { PrintError("VMCB Unable to be written\n"); @@ -456,6 +546,11 @@ static int save_core(struct guest_info * info, struct v3_chkpt * chkpt) { snprintf(key_name, 16, "vmcs_data%d", info->vcpu_id); ctx = v3_chkpt_open_ctx(chkpt, NULL, key_name); + + if (!ctx) { + PrintError("Could not open context to store VMX core\n"); + return -1; + } if (v3_vmx_save_core(info, ctx) == -1) { PrintError("VMX checkpoint failed\n"); @@ -484,7 +579,7 @@ int v3_chkpt_save_vm(struct v3_vm_info * vm, char * store, char * url) { chkpt = chkpt_open(vm, store, url, SAVE); if (chkpt == NULL) { - PrintError("Error creating checkpoint store\n"); + PrintError("Error creating checkpoint store for url %s\n",url); return -1; } diff --git a/palacios/src/palacios/vmm_chkpt_stores.h b/palacios/src/palacios/vmm_chkpt_stores.h index 389a0ca..3c2572b 100644 --- a/palacios/src/palacios/vmm_chkpt_stores.h +++ b/palacios/src/palacios/vmm_chkpt_stores.h @@ -152,12 +152,20 @@ static int keyed_stream_close_ctx(void * store_data, void * ctx) { static int keyed_stream_save(void * store_data, void * ctx, char * tag, uint64_t len, void * buf) { - return v3_keyed_stream_write_key(store_data, ctx, buf, len); + if (v3_keyed_stream_write_key(store_data, ctx, buf, len) != len) { + return -1; + } else { + return 0; + } } static int keyed_stream_load(void * store_data, void * ctx, char * tag, uint64_t len, void * buf) { - return v3_keyed_stream_read_key(store_data, ctx, buf, len); + if (v3_keyed_stream_read_key(store_data, ctx, buf, len) != len) { + return -1; + } else { + return 0; + } } diff --git a/palacios/src/palacios/vmm_dev_mgr.c b/palacios/src/palacios/vmm_dev_mgr.c index c79e162..b045de9 100644 --- a/palacios/src/palacios/vmm_dev_mgr.c +++ b/palacios/src/palacios/vmm_dev_mgr.c @@ -144,21 +144,44 @@ int v3_save_vm_devices(struct v3_vm_info * vm, struct v3_chkpt * chkpt) { name_table = V3_Malloc(table_len); + if (!name_table) { + PrintError("Unable to allocate space in device manager save\n"); + return -1; + } + memset(name_table, 0, table_len); - dev_mgr_ctx = v3_chkpt_open_ctx(chkpt, NULL, "devices"); + if (!dev_mgr_ctx) { + PrintError("Unable to open device manager context\n"); + V3_Free(name_table); + return -1; + } + list_for_each_entry(dev, &(mgr->dev_list), dev_link) { if (dev->ops->save) { strncpy(name_table + tbl_offset, dev->name, 32); tbl_offset += 32; num_saved_devs++; - } + } else { + PrintDebug("Skipping device %s\n"); + } + } + + if (v3_chkpt_save(dev_mgr_ctx, "num_devs", 4, &num_saved_devs) == -1) { + PrintError("Unable to store num_devs\n"); + v3_chkpt_close_ctx(dev_mgr_ctx); + V3_Free(name_table); + return -1; } - v3_chkpt_save(dev_mgr_ctx, "num_devs", 4, &num_saved_devs); - v3_chkpt_save(dev_mgr_ctx, "names", num_saved_devs*32, name_table); + if (v3_chkpt_save(dev_mgr_ctx, "names", num_saved_devs*32, name_table) == -1) { + PrintError("Unable to store names of devices\n"); + v3_chkpt_close_ctx(dev_mgr_ctx); + V3_Free(name_table); + return -1; + } v3_chkpt_close_ctx(dev_mgr_ctx); @@ -171,8 +194,17 @@ int v3_save_vm_devices(struct v3_vm_info * vm, struct v3_chkpt * chkpt) { V3_Print("Saving state for device (%s)\n", dev->name); dev_ctx = v3_chkpt_open_ctx(chkpt, NULL, dev->name); + + if (!dev_ctx) { + PrintError("Unable to open context for device %s\n",dev->name); + return -1; + } - dev->ops->save(dev_ctx, dev->private_data); + if (dev->ops->save(dev_ctx, dev->private_data)) { + PrintError("Unable t save device %s\n",dev->name); + v3_chkpt_close_ctx(dev_ctx); + return -1; + } v3_chkpt_close_ctx(dev_ctx); @@ -195,13 +227,32 @@ int v3_load_vm_devices(struct v3_vm_info * vm, struct v3_chkpt * chkpt) { dev_mgr_ctx = v3_chkpt_open_ctx(chkpt, NULL, "devices"); - v3_chkpt_load(dev_mgr_ctx, "num_devs", 4, &num_devs); + if (!dev_mgr_ctx) { + PrintError("Unable to open devices for load\n"); + return -1; + } + + if (v3_chkpt_load(dev_mgr_ctx, "num_devs", 4, &num_devs) == -1) { + PrintError("Unable to load num_devs\n"); + v3_chkpt_close_ctx(dev_mgr_ctx); + return -1; + } V3_Print("Loading State for %d devices\n", num_devs); name_table = V3_Malloc(32 * num_devs); + + if (!name_table) { + PrintError("Unable to allocate space for device table\n"); + v3_chkpt_close_ctx(dev_mgr_ctx); + return -1; + } - v3_chkpt_load(dev_mgr_ctx, "names", 32 * num_devs, name_table); + if (v3_chkpt_load(dev_mgr_ctx, "names", 32 * num_devs, name_table) == -1) { + PrintError("Unable to load device name table\n"); + v3_chkpt_close_ctx(dev_mgr_ctx); + V3_Free(name_table); + } v3_chkpt_close_ctx(dev_mgr_ctx); @@ -228,7 +279,9 @@ int v3_load_vm_devices(struct v3_vm_info * vm, struct v3_chkpt * chkpt) { } - dev->ops->load(dev_ctx, dev->private_data); + if (dev->ops->load(dev_ctx, dev->private_data)) { + PrintError("Load of device %s failed\n",name); + } v3_chkpt_close_ctx(dev_ctx); } @@ -250,8 +303,6 @@ int v3_deinit_dev_mgr(struct v3_vm_info * vm) { free_frontends(vm, mgr); - - v3_free_htable(mgr->dev_table, 0, 0); return 0; diff --git a/palacios/src/palacios/vmx.c b/palacios/src/palacios/vmx.c index 8d13c11..649181b 100644 --- a/palacios/src/palacios/vmx.c +++ b/palacios/src/palacios/vmx.c @@ -643,8 +643,11 @@ int v3_vmx_save_core(struct guest_info * core, void * ctx){ struct vmx_data * vmx_info = (struct vmx_data *)(core->vmm_data); // note that the vmcs pointer is an HPA, but we need an HVA - v3_chkpt_save(ctx, "vmcs_data", PAGE_SIZE_4KB, V3_VAddr((void*) - (vmx_info->vmcs_ptr_phys))); + if (v3_chkpt_save(ctx, "vmcs_data", PAGE_SIZE_4KB, + V3_VAddr((void*) (vmx_info->vmcs_ptr_phys))) ==-1) { + PrintError("Could not save vmcs data for VMX\n"); + return -1; + } return 0; } @@ -655,8 +658,17 @@ int v3_vmx_load_core(struct guest_info * core, void * ctx){ addr_t vmcs_page_paddr; //HPA vmcs_page_paddr = (addr_t) V3_AllocPages(1); + + if (!vmcs_page_paddr) { + PrintError("Could not allocate space for a vmcs in VMX\n"); + return -1; + } - v3_chkpt_load(ctx, "vmcs_data", PAGE_SIZE_4KB, V3_VAddr((void *)vmcs_page_paddr)); + if (v3_chkpt_load(ctx, "vmcs_data", PAGE_SIZE_4KB, + V3_VAddr((void *)vmcs_page_paddr)) == -1) { + PrintError("Could not load vmcs data for VMX\n"); + return -1; + } vmcs_clear(vmx_info->vmcs_ptr_phys);