[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 5/8] libxl: Share logic for finding path between qemuu and pygrub
George Dunlap writes ("[PATCH 5/8] libxl: Share logic for finding path between qemuu and pygrub"): > From: George Dunlap <george.dunlap@xxxxxxxxxxxxx> Thanks. There are a few format inconsistencies (long lines, spaces) which I won't enumerate. > * In the case of a block script, or a non-dom0 backend, qemuu will now > print a warning and skip the disk, rather than adding bogus > parameters that will cause qemuu to error out. I think the consequences here ought to be better spelled out. AFAICT the implication is that, in this case, the disk will be available to the guest as a PV disk (because libxl will select a backend other than qdisk) but not via the emulated IDE ? IWBNI the resulting functional restriction were documented, but I won't insist on that given that you're improving matters. > @@ -3023,6 +3024,16 @@ static char * > libxl__device_disk_find_local_path(libxl__gc *gc, > goto out; > } > > + /* > + * If we're being called for a qemu path, we can pass the target > + * string directly as well > + */ > + if (qdisk_direct && disk->backend == LIBXL_DISK_BACKEND_QDISK ) { > + path = libxl__strdup(gc, disk->pdev_path); > + LOG(DEBUG, "Directly accessing local QDISK target %s", path); > + goto out; Is this really true ? What if the format is qcow2 ? I think that might result in feeding pygrub the qcow2 file rather than the virtual block image it contains. Overall I think this patch is otherwise probably good but it it's a bit hard to see the wood for the trees. If you felt like factoring out some of the refactoring and variable renaming, from the functional change, that would be very nice. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |