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

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:

* 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.

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.

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