Re: [virt-tools-list] [PATCH 2/2] Fix spice includes

2015-11-11 Thread Eduardo Lima (Etrunko)
On 11/11/2015 05:11 AM, Pavel Grunt wrote:
> Hi Eduardo,
> 
> these warnings were introduced after spice-gtk v0.30 release, virt-viewer
> currently depends on spice-gtk v0.29.35. You can make the code conditional or
> wait for the spice-gtk v0.31 release.
> 

Alright, I guess it is better to wait then.

Regards. Eduardo.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH 1/2] Remove unused 'window-removed' signal

2015-11-11 Thread Pavel Grunt
Ok, ack.

Pavel
On Tue, 2015-11-10 at 18:39 -0200, Eduardo Lima (Etrunko) wrote:
> Signed-off-by: Eduardo Lima (Etrunko) 
> ---
>  src/virt-viewer-app.c | 14 --
>  src/virt-viewer-app.h |  1 -
>  2 files changed, 15 deletions(-)
> 
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 653b30c..b14b509 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -176,7 +176,6 @@ enum {
>  
>  enum {
>  SIGNAL_WINDOW_ADDED,
> -SIGNAL_WINDOW_REMOVED,
>  SIGNAL_LAST,
>  };
>  
> @@ -1046,8 +1045,6 @@ static void
> virt_viewer_app_remove_nth_window(VirtViewerApp *self,
>  g_debug("Remove window %d %p", nth, win);
>  self->priv->windows = g_list_remove(self->priv->windows, win);
>  
> -g_signal_emit(self, signals[SIGNAL_WINDOW_REMOVED], 0, win);
> -
>  g_object_unref(win);
>  }
>  
> @@ -2003,17 +2000,6 @@ virt_viewer_app_class_init (VirtViewerAppClass *klass)
>   G_TYPE_NONE,
>   1,
>   G_TYPE_OBJECT);
> -
> -signals[SIGNAL_WINDOW_REMOVED] =
> -g_signal_new("window-removed",
> - G_OBJECT_CLASS_TYPE(object_class),
> - G_SIGNAL_RUN_LAST,
> - G_STRUCT_OFFSET(VirtViewerAppClass, window_removed),
> - NULL, NULL,
> - g_cclosure_marshal_VOID__OBJECT,
> - G_TYPE_NONE,
> - 1,
> - G_TYPE_OBJECT);
>  }
>  
>  void
> diff --git a/src/virt-viewer-app.h b/src/virt-viewer-app.h
> index bbbc9b4..7aa315d 100644
> --- a/src/virt-viewer-app.h
> +++ b/src/virt-viewer-app.h
> @@ -48,7 +48,6 @@ typedef struct {
>  
>  /* signals */
>  void (*window_added) (VirtViewerApp *self, VirtViewerWindow *window);
> -void (*window_removed) (VirtViewerApp *self, VirtViewerWindow *window);
>  
>  /*< private >*/
>  gboolean (*start) (VirtViewerApp *self, GError **error);

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [virt-manager PATCH] virt-install: don't report missing console in extra-args for ppc64

2015-11-11 Thread Pavel Hrdina
On Tue, Nov 10, 2015 at 04:20:51PM -0500, Cole Robinson wrote:
> On 11/09/2015 02:02 PM, Pavel Hrdina wrote:
> > Kernel for ppc64 automatically enables serial console, there is no need
> > to report any warning.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1247434
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> > 
> > Pushed as trivial.
> > 
> >  virt-install | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/virt-install b/virt-install
> > index d509256..3656835 100755
> > --- a/virt-install
> > +++ b/virt-install
> > @@ -520,6 +520,8 @@ def _show_nographics_warnings(options, guest):
> >  console_type = serial_arm_arg
> >  if guest.get_devices("console")[0].target_type in ["virtio", "xen"]:
> >  console_type = hvc_arg
> > +if guest.os.is_ppc64():
> > +return
> >  
> >  if console_type in (options.extra_args or ""):
> >  return
> > 
> 
> I don't think this actually works as you expect, this check is done _after_
> the warning mentioned in the bug report. I think it should be moved near the
> top of the function, where the similar arm machvirt check is done
> 
> - Cole

It works as I expected, the warning mentioned in the BZ was updated by
commit 601a82cb.  I don't want to return near the top of the function where is
that check for arm, because there is still another warning about "No --console
device added, ..." which should be printed if user explicitly disable console
for his guest using "--console none".

Pavel

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-manager v2] details: show Channel label by device type

2015-11-11 Thread Cole Robinson
On 11/11/2015 07:26 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> Currently we show channel label by its name.
> If we use name com.redhat.spice.0 but set it
> as unix socket, the label in details keep unchanged.
> 
> This patch will set label according to device type
> if we failed matching target_name
> 
> Signed-off-by: Chen Hanxiao 
> ---
> v2: use device type when failed to match target name.
> 
>  virtManager/details.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/virtManager/details.py b/virtManager/details.py
> index 71ba39d..d0e8dc8 100644
> --- a/virtManager/details.py
> +++ b/virtManager/details.py
> @@ -214,6 +214,8 @@ def _label_for_device(dev):
>  if devtype == "channel":
>  label = _("Channel")
>  name = dev.pretty_channel_name(dev.target_name)
> +if not name:
> +name = dev.pretty_type(dev.type)
>  if name:
>  label += " %s" % name
>  return label
> 

Ah okay that looks fine, ACK

- Cole

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-manager PATCH] virt-install: refresh storage pool after disk is created

2015-11-11 Thread Pavel Hrdina
On Tue, Nov 10, 2015 at 04:51:25PM -0500, Cole Robinson wrote:
> On 11/10/2015 08:19 AM, Pavel Hrdina wrote:
> > This fixes an issue where you need to manually refresh storage pool or
> > restart virt-manager in order to be able delete guest that is currently
> > installed.  This affect remote connection and non-root users.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1251400
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  virtinst/storage.py | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/virtinst/storage.py b/virtinst/storage.py
> > index e50b78f..21d21a6 100644
> > --- a/virtinst/storage.py
> > +++ b/virtinst/storage.py
> > @@ -797,6 +797,8 @@ class StorageVolume(_StorageObject):
> >  logging.debug("Using vol create flags=%s", createflags)
> >  vol = self.pool.createXML(xml, createflags)
> >  
> > +self.pool.refresh()
> > +
> >  self._install_finished = True
> >  t.join()
> >  meter.end(self.capacity)
> > 
> 
> I don't think this patch actually fixes the reported issue... it doesn't do
> anything for me at least.
> 
> The root problem here is that libvirt doesn't support event APIs for storage
> operations, and virt-manager no longer relentlessly polls the storage list
> since it sucks for high latency connections. The disk image is created behind
> virt-managers back, and nothing in virt-manager is catching that it's updated.
> 
> Particularly the pool.refresh() call here isn't going to signal virt-manager
> in any way to update its internal pool/vol cache, so it won't be reflected in
> the app.

Ouch, you are right, I've probably had that disk in virt-manager's cache and
that's why I thought, that was a correct fix.

> 
> We already have some hacks in place to deal with outdated cache in
> virt-manager, like we will request a pool/vol cache update when launching the
> new VM wizard. But in this particular case I don't think it's important enough
> to try and fix, we should just defer it until libvirt has storage event APIs.

Yes, we need to wait for those events and it's obvious that virt-install cannot
affect virt-manager's cache.

Thanks for review,

Pavel

> 
> Thanks,
> Cole

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-manager PATCH] ui: improve pause/resume tooltip

2015-11-11 Thread Pavel Hrdina
On Tue, Nov 10, 2015 at 04:23:35PM -0500, Cole Robinson wrote:
> On 11/09/2015 02:06 PM, Pavel Hrdina wrote:
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1238618
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  ui/details.ui | 2 +-
> >  ui/manager.ui | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/ui/details.ui b/ui/details.ui
> > index 6edc0e4..99718d2 100644
> > --- a/ui/details.ui
> > +++ b/ui/details.ui
> > @@ -464,7 +464,7 @@
> >
> >  True
> >  False
> > -Pause 
> > the virtual machine
> > + > translatable="yes">Pause/resume the virtual machine
> >   > translatable="yes">Pause
> >  True
> >  gtk-media-pause
> > diff --git a/ui/manager.ui b/ui/manager.ui
> > index 2f3d465..63a4794 100644
> > --- a/ui/manager.ui
> > +++ b/ui/manager.ui
> > @@ -340,7 +340,7 @@
> >  True
> >  False
> >  True
> > -Pause 
> > the virtual machine
> > + > translatable="yes">Pause/resume the virtual machine
> >   > translatable="yes">_Pause
> >  True
> >  gtk-media-pause
> > 
> 
> It's a bit more work, but I prefer this done in the code, so a different
> tooltip is set depending on if the VM is paused or not.
> details.py:refresh_vm_state and manager.py:update_current_selection
> 
> Thanks,
> Cole

Yes, that makes sense, I've just tried to avoid that solution :)

Pavel

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [virt-manager v2] details: show Channel label by device type

2015-11-11 Thread Chen Hanxiao
From: Chen Hanxiao 

Currently we show channel label by its name.
If we use name com.redhat.spice.0 but set it
as unix socket, the label in details keep unchanged.

This patch will set label according to device type
if we failed matching target_name

Signed-off-by: Chen Hanxiao 
---
v2: use device type when failed to match target name.

 virtManager/details.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/virtManager/details.py b/virtManager/details.py
index 71ba39d..d0e8dc8 100644
--- a/virtManager/details.py
+++ b/virtManager/details.py
@@ -214,6 +214,8 @@ def _label_for_device(dev):
 if devtype == "channel":
 label = _("Channel")
 name = dev.pretty_channel_name(dev.target_name)
+if not name:
+name = dev.pretty_type(dev.type)
 if name:
 label += " %s" % name
 return label
-- 
1.9.0


___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-manager PATCH] virt-install: don't report missing console in extra-args for ppc64

2015-11-11 Thread Cole Robinson
On 11/11/2015 08:17 AM, Pavel Hrdina wrote:
> On Tue, Nov 10, 2015 at 04:20:51PM -0500, Cole Robinson wrote:
>> On 11/09/2015 02:02 PM, Pavel Hrdina wrote:
>>> Kernel for ppc64 automatically enables serial console, there is no need
>>> to report any warning.
>>>
>>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1247434
>>>
>>> Signed-off-by: Pavel Hrdina 
>>> ---
>>>
>>> Pushed as trivial.
>>>
>>>  virt-install | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/virt-install b/virt-install
>>> index d509256..3656835 100755
>>> --- a/virt-install
>>> +++ b/virt-install
>>> @@ -520,6 +520,8 @@ def _show_nographics_warnings(options, guest):
>>>  console_type = serial_arm_arg
>>>  if guest.get_devices("console")[0].target_type in ["virtio", "xen"]:
>>>  console_type = hvc_arg
>>> +if guest.os.is_ppc64():
>>> +return
>>>  
>>>  if console_type in (options.extra_args or ""):
>>>  return
>>>
>>
>> I don't think this actually works as you expect, this check is done _after_
>> the warning mentioned in the bug report. I think it should be moved near the
>> top of the function, where the similar arm machvirt check is done
>>
>> - Cole
> 
> It works as I expected, the warning mentioned in the BZ was updated by
> commit 601a82cb.  I don't want to return near the top of the function where is
> that check for arm, because there is still another warning about "No --console
> device added, ..." which should be printed if user explicitly disable console
> for his guest using "--console none".
> 

You're right, my mistake! Misread the error message in the bug report.

ACK to this, but it means I need to fix the arm check as well

- Cole

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-manager PATCH 1/2] virt-install: warn user about missing console while installing on arm

2015-11-11 Thread Cole Robinson
On 11/11/2015 03:32 PM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina 
> ---
> 
> Pushed as trivial.
> 
>  virt-install | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/virt-install b/virt-install
> index 3656835..e465c46 100755
> --- a/virt-install
> +++ b/virt-install
> @@ -488,10 +488,6 @@ def _show_nographics_warnings(options, guest):
>  return
>  if not options.autoconsole:
>  return
> -if guest.os.is_arm_machvirt():
> -# Later arm kernels figure out console= automatically, so don't
> -# warn about it.
> -return
>  
>  if guest.installer.cdrom:
>  logging.warn(_("CDROM media does not print to the text console "
> @@ -520,7 +516,9 @@ def _show_nographics_warnings(options, guest):
>  console_type = serial_arm_arg
>  if guest.get_devices("console")[0].target_type in ["virtio", "xen"]:
>  console_type = hvc_arg
> -if guest.os.is_ppc64():
> +if guest.os.is_ppc64() or guest.os.is_arm_machvirt():
> +# Later arm/ppc kernels figure out console= automatically, so don't
> +# warn about it.
>  return
>  
>  if console_type in (options.extra_args or ""):
> 

ACK

- Cole

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-manager PATCH 2/2] virt-install: fix condition that detect if console is present

2015-11-11 Thread Pavel Hrdina
On Wed, Nov 11, 2015 at 03:50:30PM -0500, Cole Robinson wrote:
> On 11/11/2015 03:32 PM, Pavel Hrdina wrote:
> > Function get_devices() always returns a list, if there is no requested
> > device the list is empty.
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> > 
> > Pushed as trivial.
> > 
> >  virt-install | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/virt-install b/virt-install
> > index e465c46..c5c6e57 100755
> > --- a/virt-install
> > +++ b/virt-install
> > @@ -502,7 +502,7 @@ def _show_nographics_warnings(options, guest):
> >  # Trying --location --nographics with console connect. Warn if
> >  # they likely won't see any output.
> >  
> > -if not guest.get_devices("console"):
> > +if len(guest.get_devices("console")) = 0:
> >  logging.warn(_("No --console device added, you likely will not "
> >  "see text install output from the guest."))
> >  return
> > 
> 
> Does this actually make a difference? 'not []' returns True after all...
> 
> - Cole

Hm, that's true, I wonder, why I didn't saw that warning before.  I've pushed it
already, so should we leave it as it is or revert this patch?

Pavel

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-manager PATCH] virt-install: don't report missing console in extra-args for ppc64

2015-11-11 Thread Pavel Hrdina
On Wed, Nov 11, 2015 at 02:58:16PM -0500, Cole Robinson wrote:
> On 11/11/2015 08:17 AM, Pavel Hrdina wrote:
> > On Tue, Nov 10, 2015 at 04:20:51PM -0500, Cole Robinson wrote:
> >> On 11/09/2015 02:02 PM, Pavel Hrdina wrote:
> >>> Kernel for ppc64 automatically enables serial console, there is no need
> >>> to report any warning.
> >>>
> >>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1247434
> >>>
> >>> Signed-off-by: Pavel Hrdina 
> >>> ---
> >>>
> >>> Pushed as trivial.
> >>>
> >>>  virt-install | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/virt-install b/virt-install
> >>> index d509256..3656835 100755
> >>> --- a/virt-install
> >>> +++ b/virt-install
> >>> @@ -520,6 +520,8 @@ def _show_nographics_warnings(options, guest):
> >>>  console_type = serial_arm_arg
> >>>  if guest.get_devices("console")[0].target_type in ["virtio", "xen"]:
> >>>  console_type = hvc_arg
> >>> +if guest.os.is_ppc64():
> >>> +return
> >>>  
> >>>  if console_type in (options.extra_args or ""):
> >>>  return
> >>>
> >>
> >> I don't think this actually works as you expect, this check is done _after_
> >> the warning mentioned in the bug report. I think it should be moved near 
> >> the
> >> top of the function, where the similar arm machvirt check is done
> >>
> >> - Cole
> > 
> > It works as I expected, the warning mentioned in the BZ was updated by
> > commit 601a82cb.  I don't want to return near the top of the function where 
> > is
> > that check for arm, because there is still another warning about "No 
> > --console
> > device added, ..." which should be printed if user explicitly disable 
> > console
> > for his guest using "--console none".
> > 
> 
> You're right, my mistake! Misread the error message in the bug report.
> 
> ACK to this, but it means I need to fix the arm check as well
> 
> - Cole
> 

Thanks

That's right, I didn't notice the arm condition at first and there is one more
thing that needs to be fixed, I'll push it in a minute.

Pavel

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [virt-manager PATCH 0/2] some minor virt-install fixes

2015-11-11 Thread Pavel Hrdina
Pavel Hrdina (2):
  virt-install: warn user about missing console while installing on arm
  virt-install: fix condition that detect if console is present

 virt-install | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

-- 
2.6.3

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [virt-manager PATCH 2/2] virt-install: fix condition that detect if console is present

2015-11-11 Thread Pavel Hrdina
Function get_devices() always returns a list, if there is no requested
device the list is empty.

Signed-off-by: Pavel Hrdina 
---

Pushed as trivial.

 virt-install | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/virt-install b/virt-install
index e465c46..c5c6e57 100755
--- a/virt-install
+++ b/virt-install
@@ -502,7 +502,7 @@ def _show_nographics_warnings(options, guest):
 # Trying --location --nographics with console connect. Warn if
 # they likely won't see any output.
 
-if not guest.get_devices("console"):
+if len(guest.get_devices("console")) = 0:
 logging.warn(_("No --console device added, you likely will not "
 "see text install output from the guest."))
 return
-- 
2.6.3

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [virt-manager PATCH 1/2] virt-install: warn user about missing console while installing on arm

2015-11-11 Thread Pavel Hrdina
Signed-off-by: Pavel Hrdina 
---

Pushed as trivial.

 virt-install | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/virt-install b/virt-install
index 3656835..e465c46 100755
--- a/virt-install
+++ b/virt-install
@@ -488,10 +488,6 @@ def _show_nographics_warnings(options, guest):
 return
 if not options.autoconsole:
 return
-if guest.os.is_arm_machvirt():
-# Later arm kernels figure out console= automatically, so don't
-# warn about it.
-return
 
 if guest.installer.cdrom:
 logging.warn(_("CDROM media does not print to the text console "
@@ -520,7 +516,9 @@ def _show_nographics_warnings(options, guest):
 console_type = serial_arm_arg
 if guest.get_devices("console")[0].target_type in ["virtio", "xen"]:
 console_type = hvc_arg
-if guest.os.is_ppc64():
+if guest.os.is_ppc64() or guest.os.is_arm_machvirt():
+# Later arm/ppc kernels figure out console= automatically, so don't
+# warn about it.
 return
 
 if console_type in (options.extra_args or ""):
-- 
2.6.3

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-manager PATCH 2/2] virt-install: fix condition that detect if console is present

2015-11-11 Thread Cole Robinson
On 11/11/2015 03:32 PM, Pavel Hrdina wrote:
> Function get_devices() always returns a list, if there is no requested
> device the list is empty.
> 
> Signed-off-by: Pavel Hrdina 
> ---
> 
> Pushed as trivial.
> 
>  virt-install | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/virt-install b/virt-install
> index e465c46..c5c6e57 100755
> --- a/virt-install
> +++ b/virt-install
> @@ -502,7 +502,7 @@ def _show_nographics_warnings(options, guest):
>  # Trying --location --nographics with console connect. Warn if
>  # they likely won't see any output.
>  
> -if not guest.get_devices("console"):
> +if len(guest.get_devices("console")) = 0:
>  logging.warn(_("No --console device added, you likely will not "
>  "see text install output from the guest."))
>  return
> 

Does this actually make a difference? 'not []' returns True after all...

- Cole

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-manager PATCH 2/2] virt-install: fix condition that detect if console is present

2015-11-11 Thread Cole Robinson
On 11/11/2015 04:01 PM, Pavel Hrdina wrote:
> On Wed, Nov 11, 2015 at 03:50:30PM -0500, Cole Robinson wrote:
>> On 11/11/2015 03:32 PM, Pavel Hrdina wrote:
>>> Function get_devices() always returns a list, if there is no requested
>>> device the list is empty.
>>>
>>> Signed-off-by: Pavel Hrdina 
>>> ---
>>>
>>> Pushed as trivial.
>>>
>>>  virt-install | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/virt-install b/virt-install
>>> index e465c46..c5c6e57 100755
>>> --- a/virt-install
>>> +++ b/virt-install
>>> @@ -502,7 +502,7 @@ def _show_nographics_warnings(options, guest):
>>>  # Trying --location --nographics with console connect. Warn if
>>>  # they likely won't see any output.
>>>  
>>> -if not guest.get_devices("console"):
>>> +if len(guest.get_devices("console")) = 0:
>>>  logging.warn(_("No --console device added, you likely will not "
>>>  "see text install output from the guest."))
>>>  return
>>>
>>
>> Does this actually make a difference? 'not []' returns True after all...
>>
>> - Cole
> 
> Hm, that's true, I wonder, why I didn't saw that warning before.  I've pushed 
> it
> already, so should we leave it as it is or revert this patch?
> 

It's minor but I'd suggest reverting it, so that the git history indicates
it's not a bug fix. I don't want to be scratching my head 6 months from now
wondering if this actually fixed something, because I'll probably forget this
conversation :)

- Cole

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list