<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" "http://www.w3.org/TR/REC-html40/strict.dtd"> <html><head><meta name="qrichtext" content="1" /><style type="text/css"> p, li { white-space: pre-wrap; } </style></head><body style=" font-family:'Monospace'; font-size:10pt; font-weight:400; font-style:normal;"> Eric, Thank you for your comments! I'll check related to HyperV adoption commits and will try to fix necessary things in the next patch series. Links to product pages: http://www.parallels.com/products/server/baremetal/sp/resources/ http://www.parallels.com/download/server/ > Is this driver more like qemu, > where it requires talking to a daemon like libvirtd (and where remote > access is provided by libvirt), or more like esx, where it is > translating things at the client level while talking to an esx protocol > (and where remote access is provided by the hypervisor)? We have a userspace daemon on PSBM node which is responsible for incoming requests management (like libvirtd) and client library (libprl_sdk.so) for remote/local access to it. Communication protocol is XML-based over TCP. > Is it really Linux-only, or if those > programs someday exist on other platforms, have you artificially > restricted this driver? Virtualization SDK is available for 3 platforms (Linux, Windows and Mac OS X) already. I decided to restrict it initially in order to achieve something usable on the single platform first, than extend it to others. Patches for extension will be small - just remove this restriction and add load of the shared library on appropriate paths. Just want to avoid additional testing on the adoption stage. -- Thanks, Dmitry. On Tuesday, September 27, 2011 09:39:14 PM Eric Blake wrote: > On 09/26/2011 05:59 AM, Dmitry Mishin wrote: > > Parallels Server Bare Metal is a virtualization solution which allows to > > run both Containers (OpenVZ-like) and virtual machines (like any other > > hardware hypervisor). > > Any web page link so we can familiarize ourselves with the project? > > > This patch implements initial driver stub with almost no functionality > > except an ability to open and close driver. At the same time it contains > > initialization of Parallels API and clearly demonstrates supposed > > approach to future driver implementation. In particular, I suppose to > > use parallels-virtualization-sdk library provisioned with the PSBM > > distribution (and separately also) for PSBM's virtual servers > > management. > > > > Signed-off-by: Dmitry Mishin<dim@parallels.com> > > --- > > > > autobuild.sh | 1 + > > configure.ac | 21 +++++ > > include/libvirt/virterror.h | 1 + > > src/Makefile.am | 22 +++++ > > src/README | 1 + > > src/driver.h | 1 + > > src/libvirt.c | 9 ++ > > src/psbm/psbm_api.c | 117 ++++++++++++++++++++++++ > > src/psbm/psbm_api.h | 59 ++++++++++++ > > src/psbm/psbm_driver.c | 208 > > +++++++++++++++++++++++++++++++++++++++++++ src/psbm/psbm_driver.h > > | 32 +++++++ > > src/psbm/psbm_private.h | 49 ++++++++++ > > src/util/virterror.c | 3 + > > Missing a hypervisor stub page under docs/ (that would be a good place > to include the link I mentioned above). Is this driver more like qemu, > where it requires talking to a daemon like libvirtd (and where remote > access is provided by libvirt), or more like esx, where it is > translating things at the client level while talking to an esx protocol > (and where remote access is provided by the hypervisor)? > > Missing changes to libvirt.spec.in and mingw32-libvirt.spec.in for > making compilation of the new driver conditional when building for > Fedora and friends. > > I'd feel a bit more comfortable reviewing this patch if you also had a > followon patch that can do some basic APIs, such as start and stop > guests, or even just list the names of running guests. Of course, those > should be separate patches, but it is better to push a patch series that > makes the hypervisor driver useful, rather than just pushing this patch > in isolation where the hypervisor driver can't do anything at all. Look > at the recent HyperV hypervisor driver addition (around commit 5e3b0f8) > for an example of what all is needed to make this driver addition > successful. > > > + > > +if test "$with_psbm" = "yes"&& test "$with_linux" = "no"; then > > + AC_MSG_ERROR([The PSBM driver can be enabled on Linux only.]) > > Are there any libraries that have to be linked in? Is this all calls to > third-party program(s) (in which case, should configure probe for the > absolute path of that program)? Is it really Linux-only, or if those > programs someday exist on other platforms, have you artificially > restricted this driver? > > > +int psbmApiInit(struct psbm_driver *driver) > > +{ > > + const char *libname = "libprl_sdk.so"; > > Looks like you are using library calls, so configure needs to probe for > the existence of this library. > > > + > > +static virDriver psbmDriver = { > > + .no = VIR_DRV_PSBM, > > + .name = "PSBM", > > + .open = psbmOpen, /* 0.3.1 */ > > + .close = psbmClose, /* 0.3.1 */ > > + .type = psbmGetType, /* 0.3.1 */ > > + .version = psbmGetVersion, /* 0.3.1 */ > > + .nodeGetInfo = nodeGetInfo, /* 0.3.1 */ > > + .listDomains = psbmListDomains, /* 0.3.1 */ > > + .numOfDomains = psbmNumDomains, /* 0.3.1 */ > > Wrong version. You are adding this functions as of 0.9.7, not 0.3.1. > > I haven't reviewed this closely, because I'm not familiar enough with > PSBM yet, but in general, I'm in favor of adding hypervisor drivers, so > I hope to see this go somewhere. </body></html>