On Tue, May 26, 2015 at 05:57:41PM +0300, Maxim Nestratov wrote: > It is necessary to have unpolluted screen when connecting to > parallels driver via virsh. > Otherwise a lot of unexpected output one will get on the console. > > Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com&gt; > --- > src/parallels/parallels_sdk.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c > index 542e09e..b717ccf 100644 > --- a/src/parallels/parallels_sdk.c > +++ b/src/parallels/parallels_sdk.c > @@ -205,6 +205,9 @@ prlsdkInit(void) > { > PRL_RESULT ret; > > + /* Disable console output */ > + PrlApi_SwitchConsoleLogging(0); Does the SDK logging ever produce anything that would be useful when troubleshooting libvirt ? If so, you might be better to protect this with if (virGetEnvBlockSUID("LIBVIRT_PARALLELS_DEBUG") == NULL) PrlApi_SwitchConsoleLogging(0); IOW, so people can do LIBVIR_PARALLELS_DEBUG=1 virsh .... if they need to see debugging info Regards, Daniel

On Tue, May 26, 2015 at 08:04:25PM +0300, Maxim Nestratov wrote: > 26.05.2015 18:01, Daniel P. Berrange пишет: >> On Tue, May 26, 2015 at 05:57:41PM +0300, Maxim Nestratov wrote: >>> It is necessary to have unpolluted screen when connecting to >>> parallels driver via virsh. >>> Otherwise a lot of unexpected output one will get on the console. >>> >>> Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com&gt; >>> --- >>> src/parallels/parallels_sdk.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c >>> index 542e09e..b717ccf 100644 >>> --- a/src/parallels/parallels_sdk.c >>> +++ b/src/parallels/parallels_sdk.c >>> @@ -205,6 +205,9 @@ prlsdkInit(void) >>> { >>> PRL_RESULT ret; >>> + /* Disable console output */ >>> + PrlApi_SwitchConsoleLogging(0); >> Does the SDK logging ever produce anything that would be useful when >> troubleshooting libvirt ? >> >> If so, you might be better to protect this with >> >> if (virGetEnvBlockSUID("LIBVIRT_PARALLELS_DEBUG") == NULL) >> PrlApi_SwitchConsoleLogging(0); >> >> IOW, so people can do >> >> LIBVIR_PARALLELS_DEBUG=1 virsh .... >> >> if they need to see debugging info >> >> Regards, >> Daniel > Actually these debug output can be helpful but as soon as they are written > to /var/log/parallels.log by default and duplicated to the console, I think > it is not necessary to keep them in two places. > Maybe it's worth mentioning this in commit message. Ok, ack to your original patch then

Regards, Daniel

On Thu, May 28, 2015 at 11:12:38AM +0300, Dmitry Guryanov wrote: > On 05/26/2015 08:05 PM, Daniel P. Berrange wrote: >> On Tue, May 26, 2015 at 08:04:25PM +0300, Maxim Nestratov wrote: >>> 26.05.2015 18:01, Daniel P. Berrange пишет: >>>> On Tue, May 26, 2015 at 05:57:41PM +0300, Maxim Nestratov wrote: >>>>> It is necessary to have unpolluted screen when connecting to >>>>> parallels driver via virsh. >>>>> Otherwise a lot of unexpected output one will get on the console. >>>>> >>>>> Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com&gt; >>>>> --- >>>>> src/parallels/parallels_sdk.c | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c >>>>> index 542e09e..b717ccf 100644 >>>>> --- a/src/parallels/parallels_sdk.c >>>>> +++ b/src/parallels/parallels_sdk.c >>>>> @@ -205,6 +205,9 @@ prlsdkInit(void) >>>>> { >>>>> PRL_RESULT ret; >>>>> + /* Disable console output */ >>>>> + PrlApi_SwitchConsoleLogging(0); >>>> Does the SDK logging ever produce anything that would be useful when >>>> troubleshooting libvirt ? >>>> >>>> If so, you might be better to protect this with >>>> >>>> if (virGetEnvBlockSUID("LIBVIRT_PARALLELS_DEBUG") == NULL) >>>> PrlApi_SwitchConsoleLogging(0); >>>> >>>> IOW, so people can do >>>> >>>> LIBVIR_PARALLELS_DEBUG=1 virsh .... >>>> >>>> if they need to see debugging info >>>> >>>> Regards, >>>> Daniel >>> Actually these debug output can be helpful but as soon as they are written >>> to /var/log/parallels.log by default and duplicated to the console, I think >>> it is not necessary to keep them in two places. >>> Maybe it's worth mentioning this in commit message. >> Ok, ack to your original patch then > I also agree with this patch, but I'll push it later because of code freeze. This patch is trivial enough and really a bug fix, so I think you're justified in pushing it during freeze.

Regards, Daniel