Re: [libvirt] [PATCH 1/1] [RFC] Parallels Server Bare Metal driver stub

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)?

Is it really Linux-only, or if those programs someday exist on other platforms, have you artificially restricted this driver?

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(a)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.