Re: [PATCH v4 00/15] parallels: rewrite driver with parallels SDK

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tuesday 09 December 2014 19:57:51 Peter Krempa wrote:
> On 12/01/14 16:38, Dmitry Guryanov wrote:
> > This patch series replaces all code, which used prlctl command
> > to interact with parallels cloud server with calls to
> > parallels sdk functions.
> > 
> > The model of this driver remain almost the same - in creates a
> > list of virDomainObj objects on connect and then functions, which
> > returns different information get info from this list.
> 
> So, I finally went ahead and pushed this series as we agreed on the
> post-freeze approach. I have following notes though:

Thanks, Peter!

> 
> * please run make syntax-check before sending patches
>   (and make sure the cppi package is installed)
> 
>   The series contained a few patches with whitespace errors in macro
> definitions.
> 
> * please compile with  --enable-compile-warnings=error
> 
>   There were a few warnings that hinted to unused variables and one bad
> constant use.

Sorry about that.

> 
>   Additionally it will make obvious my next point...
> 
> * the parallels SDK [1] produces warnings when included
> 
>   I had to add "-Wno-strict-prototypes" to PARALLELS_SDK_CFLAGS
> otherwise the compiler would complain to mistakes in the header file. I
> didn't chase them any further though to see why.
> 
> * The parallels SDK doesn't provide a pkg-config description file
> 
> Libvirt tests for the "parallels-sdk" pkg-config module, but the default
> installation doesn't provide it. It's then extremely user-unfriendly to
> hack the paths so that it actually compiles.
> 
> Please provide that desc in the installer.
> 
> * The parallels SDK installed needs root even for unpacking
> 
> I personally don't like tools that install everything. The
> self-installer [1] has an option to just unpack the files. For this
> option the root privilege shouldn't be required ... it's just ridiulous.
> 
> 

You've installed commercial version of this SDK, we haven't prepared binary 
packages for opensource version yet, sources can be found here - 
https://github.com/CloudServer/parallels-sdk.


> Anyways, thanks for contributing. It would also be really helpful to
> provide the pkg-config file.
> 
> Peter
> 
> [1] - I used the package at:
> http://download.cloudserver.parallels.com/server/pcs/en_us/parallels/6/updat
> e8/parallels-virtualization-sdk-6.8.23687.1081632.run

-- 
Dmitry Guryanov

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]