[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts
Kamala Narasimhan writes ("[xen-devel][PATCH 2/5] Xl interface change plus changes to code it impacts"): > Attached are the changes made to xl disk related interface per earlier > discussion. Please let me know if there are further comments/issues to fix. Thanks. I have some comments of my own: > +char *libxl__device_disk_string_of_backend(libxl_disk_backend backend) ... > + switch (backend) { > + case DISK_BACKEND_QEMU: return "qdisk"; > + case DISK_BACKEND_TAPDISK2: return "tap"; > + case DISK_BACKEND_BLKBACK: return "phy"; Perhaps the backend type number constants should be _QDISK, _TAP, _PHY ? I think a function like _string_of should be a simple mapping to return the string version of the same name, not also change the name. > - if (libxl__blktap_enabled(&gc)) > + if ( libxl__blktap_enabled(&gc) && > + disk->format != DISK_BACKEND_QEMU ) Don't add whitespace inside the if's ( ). (You have done this in several places. I know that libxl isn't entirely consistent but we have a defined coding style shouldn't be making the code less consistent.) > diff -r e4406b9fb064 tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Mon Feb 07 15:04:32 2011 +0000 > +++ b/tools/libxl/xl_cmdimpl.c Mon Feb 07 11:28:10 2011 -0500 > @@ -361,9 +361,9 @@ static void printf_info(int domid, > printf("\t\t(tap\n"); > printf("\t\t\t(backend_domid %d)\n", > d_config->disks[i].backend_domid); > printf("\t\t\t(domid %d)\n", d_config->disks[i].domid); > - printf("\t\t\t(physpath %s)\n", d_config->disks[i].physpath); > - printf("\t\t\t(phystype %d)\n", d_config->disks[i].phystype); > - printf("\t\t\t(virtpath %s)\n", d_config->disks[i].virtpath); > + printf("\t\t\t(pdev_path %s)\n", d_config->disks[i].pdev_path); > + printf("\t\t\t(backend %d)\n", d_config->disks[i].backend); > + printf("\t\t\t(vdev %s)\n", d_config->disks[i].vdev); This part of the code is providing information which is intended to be parsed by callers which were written to cope with the output from xm. For backward compatibility, the previously used names and values should be output with the previously used semantics; it is OK to add new ones too with more sane semantics. I think it's also acceptable to be a bit approximate with the emulation, but simply removing the old names is not correct. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |