Re: [libvirt] [PATCH 1/4] parallels: Fix connecting to parallels driver
09.02.2015 16:02, Michal Privoznik пишет:
On 30.01.2015 20:58, Maxim Nestratov wrote:
> After 55ea7be7, failures of parallelsLoadNetworks and parallelsStorageOpen
> started to fail establishing a new connection to parallels driver.
> Before this patch we simply ignored such errors.
> This patch restores the ability to connect to parallels driver
> even if parallelsLoadNetworks fails to get information about
> Parallels Bridged networks. This can happen when it is unconfigured
> (which is a common case on developers hosts for instance) or configured
> incompletely. In any case it is not a severe error and we can ignore it.
> Failures of parallelsStorageOpen occured because we incorrectly treated
> path to VM' configuration file as a directory. Now initialization of
> parallels VM domains home directory is fixed.
>
> Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
> ---
> src/parallels/parallels_network.c | 2 +-
> src/parallels/parallels_sdk.c | 8 ++++++++
> 2 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c
> index 3e7087d..033b347 100644
> --- a/src/parallels/parallels_network.c
> +++ b/src/parallels/parallels_network.c
> @@ -304,7 +304,7 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn)
>
> net = parallelsLoadNetwork(privconn, jobj2);
> if (!net)
> - goto cleanup;
> + continue;
I'm not sure this is correct. I mean, parallelsLoadNetwork() could have
failed for so many reasons. And some of them are show stoppers (e.g. OOM).
Here I
only wanted to say that failure of loading Bridged Virtual
Network information is not a show stopper.
Maybe I had better sort out things inside parallelsLoadNetworks.
>
> }
>
> diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
> index d0d2ce2..b9d5ddf 100644
> --- a/src/parallels/parallels_sdk.c
> +++ b/src/parallels/parallels_sdk.c
> @@ -1247,6 +1247,14 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
> pret = PrlVmCfg_GetHomePath(sdkdom, pdom->home, &buflen);
> prlsdkCheckRetGoto(pret, error);
>
> + // For VMs pdom->home is actually /directory/config.pvs
> + if(!IS_CT(def)){
> + // Get rid of /config.pvs in path string
> + char *s = strrchr(pdom->home, '/');
> + if (s)
> + *s = '\0';
> + }
> +
We like comments in old way /* */, and we even like spaces around
brackets and after 'if'-s.
Ok. I'll fix it.
> if (olddom) {
> /* assign new virDomainDef without any checks */
> /* we can't use virDomainObjAssignDef, because it checks
>
Michal