On 2015/7/7 19:57, Ian Jackson wrote:
Tiejun Chen writes ("[v5][PATCH 11/16] tools/libxl: detect and avoid conflicts with
RDM"):
While building a VM, HVM domain builder provides struct hvm_info_table{}
to help hvmloader. Currently it includes two fields to construct guest
e820 table by hvmloader, low_mem_pgend and high_mem_pgend. So we should
check them to fix any conflict with RDM.
...
+ *nr_entries = 0;
+ rc = xc_reserved_device_memory_map(CTX->xch, flag, seg, bus, devfn,
+ NULL, nr_entries);
+ assert(rc <= 0);
+ /* "0" means we have no any rdm entry. */
+ if (!rc)
+ goto out;
I think the error handling here is wrong. Certainly `rc' should not
be used for a libxc return.
Nope, we don't return rc directly. In any case of error, rc is reset as
"ERROR_FAIL" finally.
See tools/libxl/CODING_STYLE. I know that much of the existing code
uses rc for libxc returns but this is deprecated, and please don't
make more of it.
+ if (errno == ENOBUFS) {
+ *xrdm = libxl__malloc(gc,
+ *nr_entries *
+ sizeof(xen_reserved_device_memory_t));
+ rc = xc_reserved_device_memory_map(CTX->xch, flag, seg, bus, devfn,
+ *xrdm, nr_entries);
This might be less code, and a bit clearer, if it were a loop rather
than two calls with the second in an if.
Sorry I can't understand completely what you want to do. But I really
agree we should improve this chunk of codes, what about this?
if (errno != ENOBUFS) {
rc = ERROR_FAIL;
goto out;
}
*xrdm = libxl__malloc(gc,
*nr_entries *
sizeof(xen_reserved_device_memory_t));
rc = xc_reserved_device_memory_map(CTX->xch, flag, seg, bus, devfn,
*xrdm, nr_entries);
if (rc)
rc = ERROR_FAIL;
out:
if (rc) {
*nr_entries = 0;
*xrdm = NULL;
LOG(ERROR, "Could not get reserved device memory maps.\n");
}
return rc;
I think this make more readable and clear.
...
+ if(d_config->rdms[i].flag == LIBXL_RDM_RESERVE_FLAG_STRICT) {
^
Missing space.
Fixed.
Thanks
Tiejun
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
