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<[email protected]>
> > ---
> > 
> >   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.
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to