Re: [virt-tools-list] virt-what and security?

2011-07-07 Thread Richard W.M. Jones 
On Wed, Jul 06, 2011 at 10:33:18AM +0100, Daniel P. Berrange wrote:
 On Wed, Jul 06, 2011 at 10:15:10AM +0100, Richard W.M. Jones wrote:
  On Tue, Jul 05, 2011 at 10:06:01PM -0700, Stephen Hemminger wrote:
  [...]
   Why can lscpu find the same information without being root?
   Most of the checks (cpuid, file locations etc) can be found out
   by non-root. Only dmidecode seems to require trust, aren't there
   enough ways to find out without using dmidecode?
  
  Yes, we can probably make virt-what run as non-root, although some
  tests (the ones relying on dmidecode) will have to be disabled.
 
 On more recent kernels, some of the DMI information is also available
 unprivileged under /sys/devices/virtual/dmi/, so you may only need to
 run the dmidecode binary on older guests

Thanks Stephen, Daniel.

I am tracking this issue in the following bug:

https://bugzilla.redhat.com/show_bug.cgi?id=719611

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v

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

Re: [virt-tools-list] [python-virtinst 2/2] virt-install: Create OS dict from libosinfo

2011-07-07 Thread Cole Robinson 
On 07/01/2011 04:32 PM, Zeeshan Ali (Khattak) wrote:
 From: Zeeshan Ali (Khattak) zeesha...@gnome.org
 
 Issues:
 
 * Some info is still hard-coded
 
 * We lose the following information:
   * distro
   * sortby
 
 * I get the following error from pylint:
 
   E: 25: No name 'Libosinfo' in module 'gi.repository'
 
   Apparently pylint doesn't know about how introspection works.
 
 * libosinfo is not yet in a stage that we can add a dependency on
   it so this patch must not be merged into git master yet.
 ---
  virtinst/osdict.py |  401 
 +---
  1 files changed, 100 insertions(+), 301 deletions(-)


Any idea if gobject introspection is in RHEL6? If it isn't, what would
it take to get libosinfo + python bindings in RHEL6?

I'm afraid of deleting the existing OS_TYPES data if we won't ever have
libosinfo bindings in RHEL6, since if RHEL5 is any indication we will be
rebasing python-virtinst for quite a few RHEL6 releases, and a hard
dependency on libosinfo could cause pain later.

 diff --git a/virtinst/osdict.py b/virtinst/osdict.py
 index a520faa..040bd24 100644
 --- a/virtinst/osdict.py
 +++ b/virtinst/osdict.py
 @@ -22,6 +22,7 @@
  import support
  from VirtualDevice import VirtualDevice
  from virtinst import _virtinst as _
 +from gi.repository import Libosinfo as libosinfo
  
  HV_ALL = all
  
 @@ -36,31 +37,42 @@ INPUT = VirtualDevice.VIRTUAL_DEV_INPUT
  SOUND = VirtualDevice.VIRTUAL_DEV_AUDIO
  VIDEO = VirtualDevice.VIRTUAL_DEV_VIDEO
  
 -VIRTIO_DISK = {
 -bus : [
 -(support.SUPPORT_CONN_HV_VIRTIO, virtio),
 -]
 -}
 -
 -VIRTIO_NET = {
 -model : [
 -(support.SUPPORT_CONN_HV_VIRTIO, virtio),
 -]
 -}
 -
 -USB_TABLET = {
 -type : [
 -(HV_ALL, tablet),
 -],
 -bus  : [
 -(HV_ALL, usb),
 -]
 -}
 -
 -VGA_VIDEO = {
 -model_type: [
 -(HV_ALL, vga),
 -]
 +DEVICES = {
 +http://pciids.sourceforge.net/v2.2/pci.ids/1af4/1001: {
 +bus : [
 +(support.SUPPORT_CONN_HV_VIRTIO, virtio),
 +]
 +},
 +http://pciids.sourceforge.net/v2.2/pci.ids/1af4/1000: {
 +model : [
 +(support.SUPPORT_CONN_HV_VIRTIO, virtio),
 +]
 +},
 +http://www.linux-usb.org/usb.ids/80ee/0021: {
 +type : [
 +(HV_ALL, tablet),
 +],
 +bus  : [
 +(HV_ALL, usb),
 +]
 +},
 +http://pciids.sourceforge.net/v2.2/pci.ids/1234/: {
 +model_type: [
 +(HV_ALL, vga),
 +]
 +},
 +http://pciids.sourceforge.net/v2.2/pci.ids/10ec/8029: {
 +model : [ (HV_ALL, ne2k_pci)
 +]
 +},
 +http://pciids.sourceforge.net/v2.2/pci.ids/8086/100e: {
 +model : [ (HV_ALL, e1000)
 +]
 +},
 +http://pciids.sourceforge.net/v2.2/pci.ids/1022/2000: {
 +model : [ (HV_ALL, pcnet)
 +]
 +}
  }
  

Would be nice here to have a comment for each entry indicating what
device type it maps to in the guest XML, it was a bit more clear before.
Example:

   # virtio network
   http://pciids.sourceforge.net/v2.2/pci.ids/1af4/1000: {
   model : [
   (support.SUPPORT_CONN_HV_VIRTIO, virtio),
   ]
   },

Otherwise looks fine, provided existing virtinst tests pass, and python
setup.py sdist doesn't generate a different OS list in the man page.

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] [PATCH 1/4] Add inspection thread.

2011-07-07 Thread Cole Robinson 
On 06/30/2011 04:02 AM, Richard W.M. Jones wrote:
 From: Richard W.M. Jones rjo...@redhat.com
 
 This commit adds an inspection daemon thread which performs libguestfs
 inspection on domains when they first appear in the manager UI.
 
 python-guestfs is not required.
 ---
  src/virtManager/config.py |   25 ++
  src/virtManager/engine.py |   15 
  src/virtManager/inspection.py |  182 
 +
  src/virtManager/manager.py|3 +
  4 files changed, 225 insertions(+), 0 deletions(-)
  create mode 100644 src/virtManager/inspection.py
 
 diff --git a/src/virtManager/config.py b/src/virtManager/config.py
 index 791ef4d..d3c3155 100644
 --- a/src/virtManager/config.py
 +++ b/src/virtManager/config.py
 @@ -113,6 +113,31 @@ class vmmConfig(object):
  self._objects = []
  
  self.support_threading = virtinst.support.support_threading()
 +
 +# Check that we have the guestfs module with the non-broken
 +# support for Python threads.  Also libvirt must be
 +# thread-safe too.
 +self.support_inspection = False
 +if self.support_threading:
 +try:
 +from guestfs import GuestFS
 +g = GuestFS()
 +version = g.version()
 +if version[major] == 1: # major must be 1
 +if version[minor] == 8:
 +if version[release] = 6: # = 1.8.6
 +self.support_inspection = True
 +elif version[minor] == 10:
 +if version[release] = 1: # = 1.10.1
 +self.support_inspection = True
 +elif version[minor] == 11:
 +if version[release] = 2: # = 1.11.2
 +self.support_inspection = True
 +elif version[minor] = 12:# = 1.12.x
 +self.support_inspection = True
 +except:
 +pass
 +
  self._spice_error = None
  

Minor, but could you break this logic out into a separate function

self.support_inspection = support_inspection(self.support_threading)

  self.status_icons = {
 diff --git a/src/virtManager/engine.py b/src/virtManager/engine.py
 index 3ae1f7a..3d0cbff 100644
 --- a/src/virtManager/engine.py
 +++ b/src/virtManager/engine.py
 @@ -236,6 +236,9 @@ class vmmEngine(vmmGObject):
  if not self.config.support_threading:
  logging.debug(Libvirt doesn't support threading, skipping.)
  
 +self.inspection_thread = None
 +self._create_inspection_thread()
 +
  # Counter keeping track of how many manager and details windows
  # are open. When it is decremented to 0, close the app or
  # keep running in system tray if enabled
 @@ -529,6 +532,18 @@ class vmmEngine(vmmGObject):
  logging.debug(Exiting app normally.)
  gtk.main_quit()
  
 +def _create_inspection_thread(self):
 +if not self.config.support_inspection:
 +logging.debug(No inspection thread because 
 +  libguestfs is too old, not available, 
 +  or libvirt is not thread safe.)
 +return
 +from virtManager.inspection import vmmInspection
 +self.inspection_thread = vmmInspection()
 +self.inspection_thread.daemon = True
 +self.inspection_thread.start()
 +return
 +
  def add_connection(self, uri, readOnly=None, autoconnect=False):
  conn = self._check_connection(uri)
  if conn:
 diff --git a/src/virtManager/inspection.py b/src/virtManager/inspection.py
 new file mode 100644
 index 000..146e021
 --- /dev/null
 +++ b/src/virtManager/inspection.py
 @@ -0,0 +1,182 @@
 +#
 +# Copyright (C) 2011 Red Hat, Inc.
 +#
 +# This program is free software; you can redistribute it and/or modify
 +# it under the terms of the GNU General Public License as published by
 +# the Free Software Foundation; either version 2 of the License, or
 +# (at your option) any later version.
 +#
 +# This program is distributed in the hope that it will be useful,
 +# but WITHOUT ANY WARRANTY; without even the implied warranty of
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 +# GNU General Public License for more details.
 +#
 +# You should have received a copy of the GNU General Public License
 +# along with this program; if not, write to the Free Software
 +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
 +# MA 02110-1301 USA.
 +#
 +
 +import sys
 +import time
 +import traceback
 +from Queue import Queue
 +from threading import Thread
 +
 +from guestfs import GuestFS
 +
 +import logging
 +import util
 +
 +class vmmInspection(Thread):
 +_name = inspection thread
 +_wait = 15 # seconds
 +
 +def __init__(self):
 +Thread.__init__(self, name=self._name)
 +self._q = Queue()
 +

Re: [virt-tools-list] [PATCH 2/4] Add inspection data to the domain object.

2011-07-07 Thread Cole Robinson 
On 06/30/2011 04:02 AM, Richard W.M. Jones wrote:
 From: Richard W.M. Jones rjo...@redhat.com
 
 This adds the inspection data to the domain object, and passes the
 data up from the inspection thread.
 
 Also an inspection-changed signal has been added so that UI elements
 can find out when inspection has finished for a particular VM and
 update the UI.
 ---
  src/virtManager/domain.py |   53 
 +
  src/virtManager/inspection.py |   13 ++
  2 files changed, 66 insertions(+), 0 deletions(-)
 
 diff --git a/src/virtManager/domain.py b/src/virtManager/domain.py
 index 1e68263..0645ebb 100644
 --- a/src/virtManager/domain.py
 +++ b/src/virtManager/domain.py
 @@ -162,6 +162,16 @@ class vmmDomain(vmmLibvirtObject):
  self._stats_disk_supported = True
  self._stats_disk_skip = []
  
 +self.inspection_type = None
 +self.inspection_distro = None
 +self.inspection_major_version = None
 +self.inspection_minor_version = None
 +self.inspection_hostname = None
 +self.inspection_product_name = None
 +self.inspection_product_variant = None
 +self.inspection_icon = None
 +self.inspection_applications = None
 +
  if isinstance(self._backend, virtinst.Guest):
  return
 

I think I'd prefer if you stuck all this data in its own class:

class vmmInspectionData(object):
self._type = None
self._distro = None
...

Then use python properties for the getter/setters, since this is mostly
static data:

def _set_type(self, new):
self._type = str(new)
def _get_type(self):
return self._type
type = property(_get_type, _set_type)

Then callers just do:

dom.inspection.distro = fedora12
print dom.inspection.distro

Thanks,
Cole

 @@ -1376,6 +1386,48 @@ class vmmDomain(vmmLibvirtObject):
  return self.config.get_pervm(self.connection.get_uri(), self.uuid,
   self.config.get_details_window_size)
  
 +# Information from libguestfs inspection.
 +def set_inspection_type(self, new):
 +self.inspection_type = str(new)
 +def set_inspection_distro(self, new):
 +self.inspection_distro = str(new)
 +def set_inspection_major_version(self, new):
 +self.inspection_major_version = int(new)
 +def set_inspection_minor_version(self, new):
 +self.inspection_minor_version = int(new)
 +def set_inspection_hostname(self, new):
 +self.inspection_hostname = str(new)
 +def set_inspection_product_name(self, new):
 +self.inspection_product_name = str(new)
 +def set_inspection_product_variant(self, new):
 +self.inspection_product_variant = str(new)
 +def set_inspection_icon(self, new):
 +self.inspection_icon = str(new)
 +def set_inspection_applications(self, new):
 +self.inspection_applications = list(new)
 +
 +def inspection_data_updated(self):
 +self.idle_emit(inspection-changed)
 +
 +def get_inspection_type(self):
 +return self.inspection_type
 +def get_inspection_distro(self):
 +return self.inspection_distro
 +def get_inspection_major_version(self):
 +return self.inspection_major_version
 +def get_inspection_minor_version(self):
 +return self.inspection_minor_version
 +def get_inspection_hostname(self):
 +return self.inspection_hostname
 +def get_inspection_product_name(self):
 +return self.inspection_product_name
 +def get_inspection_product_variant(self):
 +return self.inspection_product_variant
 +def get_inspection_icon(self):
 +return self.inspection_icon
 +def get_inspection_applications(self):
 +return self.inspection_applications
 +
  
  ###
  # Polling helpers #
 @@ -1587,5 +1639,6 @@ class vmmDomainVirtinst(vmmDomain):
  vmmLibvirtObject.type_register(vmmDomain)
  vmmDomain.signal_new(vmmDomain, status-changed, [int, int])
  vmmDomain.signal_new(vmmDomain, resources-sampled, [])
 +vmmDomain.signal_new(vmmDomain, inspection-changed, [])
  
  vmmLibvirtObject.type_register(vmmDomainVirtinst)
 diff --git a/src/virtManager/inspection.py b/src/virtManager/inspection.py
 index 146e021..72406fe 100644
 --- a/src/virtManager/inspection.py
 +++ b/src/virtManager/inspection.py
 @@ -171,6 +171,19 @@ class vmmInspection(Thread):
  
  self._vmseen[vmuuid] = True
  
 +vm.set_inspection_type(typ)
 +vm.set_inspection_distro(distro)
 +vm.set_inspection_major_version(major_version)
 +vm.set_inspection_minor_version(minor_version)
 +vm.set_inspection_hostname(hostname)
 +vm.set_inspection_product_name(product_name)
 +
 +vm.set_inspection_product_variant(product_variant)
 +vm.set_inspection_icon(icon)
 +vm.set_inspection_applications(apps)
 +
 +vm.inspection_data_updated()
 +
  # Log what we found.

Re: [virt-tools-list] [PATCH 3/4] Display operating system inspection icons in main vmlist.

2011-07-07 Thread Cole Robinson 
On 06/30/2011 04:02 AM, Richard W.M. Jones wrote:
 From: Richard W.M. Jones rjo...@redhat.com
 
 ---
  src/virtManager/manager.py |   41 +++--
  1 files changed, 39 insertions(+), 2 deletions(-)
 
 diff --git a/src/virtManager/manager.py b/src/virtManager/manager.py
 index c7cca67..a52fd90 100644
 --- a/src/virtManager/manager.py
 +++ b/src/virtManager/manager.py
 @@ -43,6 +43,7 @@ ROW_IS_CONN_CONNECTED = 8
  ROW_IS_VM = 9
  ROW_IS_VM_RUNNING = 10
  ROW_COLOR = 11
 +ROW_INSPECTION_OS_ICON = 12
  
  # Columns in the tree view
  COL_NAME = 0
 @@ -352,9 +353,10 @@ class vmmManager(vmmGObjectUI):
  self.window.get_widget(vm-notebook).set_show_tabs(False)
  
  # Handle, name, markup, status, status icon, key/uuid, hint, is conn,
 -# is conn connected, is vm, is vm running, fg color
 +# is conn connected, is vm, is vm running, fg color, inspection icon
  model = gtk.TreeStore(object, str, str, str, gtk.gdk.Pixbuf, str, 
 str,
 -  bool, bool, bool, bool, gtk.gdk.Color)
 +  bool, bool, bool, bool, gtk.gdk.Color,
 +  gtk.gdk.Pixbuf)
  vmlist.set_model(model)
  util.tooltip_wrapper(vmlist, ROW_HINT, set_tooltip_column)
  
 @@ -384,6 +386,12 @@ class vmmManager(vmmGObjectUI):
  statusCol.add_attribute(status_icon, 'pixbuf', ROW_STATUS_ICON)
  statusCol.add_attribute(status_icon, 'visible', ROW_IS_VM)
  
 +inspection_os_icon = gtk.CellRendererPixbuf()
 +statusCol.pack_start(inspection_os_icon, False)
 +statusCol.add_attribute(inspection_os_icon, 'pixbuf',
 +ROW_INSPECTION_OS_ICON)
 +statusCol.add_attribute(inspection_os_icon, 'visible', ROW_IS_VM)
 +
  name_txt = gtk.CellRendererText()
  nameCol.pack_start(name_txt, True)
  nameCol.add_attribute(name_txt, 'markup', ROW_MARKUP)
 @@ -679,6 +687,7 @@ class vmmManager(vmmGObjectUI):
  vm.connect(status-changed, self.vm_status_changed)
  vm.connect(resources-sampled, self.vm_resources_sampled)
  vm.connect(config-changed, self.vm_resources_sampled)
 +vm.connect(inspection-changed, self.vm_inspection_changed)


Hmm, this makes me think maybe we shouldn't have a separate signal for
'inspection-changed' and just wrap this up into 'config-changed', since
from the UIs perspective it's all just domain details.

  vmlist = self.window.get_widget(vm-list)
  model = vmlist.get_model()
 @@ -747,6 +756,8 @@ class vmmManager(vmmGObjectUI):
  row.insert(ROW_IS_VM, True)
  row.insert(ROW_IS_VM_RUNNING, vm.is_active())
  row.insert(ROW_COLOR, gtk.gdk.Color(0, 0, 0))
 +row.insert(ROW_INSPECTION_OS_ICON,
 +   self.get_inspection_icon_pixbuf(vm,16,16))
  
  row[ROW_MARKUP] = self._build_vm_markup(vm, row)
  
 @@ -784,6 +795,7 @@ class vmmManager(vmmGObjectUI):
  row.insert(ROW_IS_VM, False)
  row.insert(ROW_IS_VM_RUNNING, False)
  row.insert(ROW_COLOR, self._build_conn_color(conn))
 +row.insert(ROW_INSPECTION_OS_ICON, None)
  
  _iter = model.append(None, row)
  path = model.get_path(_iter)
 @@ -883,6 +895,31 @@ class vmmManager(vmmGObjectUI):
  row[ROW_MARKUP] = self._build_vm_markup(vm, row)
  model.row_changed(row.path, row.iter)
  
 +def vm_inspection_changed(self, vm):
 +vmlist = self.window.get_widget(vm-list)
 +model = vmlist.get_model()
 +
 +if self.vm_row_key(vm) not in self.rows:
 +return
 +
 +row = self.rows[self.vm_row_key(vm)]
 +row[ROW_INSPECTION_OS_ICON] = 
 self.get_inspection_icon_pixbuf(vm,16,16)
 +model.row_changed(row.path, row.iter)
 +
 +def get_inspection_icon_pixbuf(self, vm, w, h):
 +# libguestfs gives us the PNG data as a string.
 +png_data = vm.get_inspection_icon()
 +if png_data == None:
 +return None
 +try:
 +pb = gtk.gdk.PixbufLoader(image_type=png)
 +pb.set_size(w, h)
 +pb.write (png_data)
 +pb.close()
 +return pb.get_pixbuf()
 +except:
 +return None
 +

Maybe we should do this just once when we set the data in
vmmInspectionData? If we need multiple sizes, just precreate multiple PNGs.

In general I like the idea though!

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] [PATCH 4/4] Add basic operating system inspection data to the VMM details page.

2011-07-07 Thread Cole Robinson 
On 06/30/2011 04:02 AM, Richard W.M. Jones wrote:
 From: Richard W.M. Jones rjo...@redhat.com
 
 This adds hostname, product name, and list of applications.
 ---
  src/virtManager/details.py |   62 
  src/vmm-details.glade  |  137 
 +++-
  2 files changed, 197 insertions(+), 2 deletions(-)
 
 diff --git a/src/virtManager/details.py b/src/virtManager/details.py
 index e3d7c4a..0cb798f 100644
 --- a/src/virtManager/details.py
 +++ b/src/virtManager/details.py
 @@ -575,6 +575,34 @@ class vmmDetails(vmmGObjectUI):
  buf.connect(changed, self.config_enable_apply)
  desc.set_buffer(buf)
  
 +# List of applications.
 +apps_list = self.window.get_widget(inspection-apps)
 +apps_model = gtk.ListStore (str, str, str)
 +apps_list.set_model(apps_model)
 +
 +name_col = gtk.TreeViewColumn(_(Name))
 +version_col = gtk.TreeViewColumn(_(Version))
 +summary_col = gtk.TreeViewColumn()
 +
 +apps_list.append_column(name_col)
 +apps_list.append_column(version_col)
 +apps_list.append_column(summary_col)
 +
 +name_text = gtk.CellRendererText()
 +name_col.pack_start(name_text, True)
 +name_col.add_attribute(name_text, 'text', 0)
 +name_col.set_sort_column_id(0)
 +
 +version_text = gtk.CellRendererText()
 +version_col.pack_start(version_text, True)
 +version_col.add_attribute(version_text, 'text', 1)
 +version_col.set_sort_column_id(1)
 +
 +summary_text = gtk.CellRendererText()
 +summary_col.pack_start(summary_text, True)
 +summary_col.add_attribute(summary_text, 'text', 2)
 +summary_col.set_sort_column_id(2)
 +
  # Clock combo
  clock_combo = self.window.get_widget(overview-clock-combo)
  clock_model = gtk.ListStore(str)
 @@ -2106,6 +2134,40 @@ class vmmDetails(vmmGObjectUI):
  self.window.get_widget(overview-arch).set_text(arch)
  self.window.get_widget(overview-emulator).set_text(emu)
  
 +# Operating System (ie. inspection data)
 +hostname = self.vm.get_inspection_hostname()
 +if not hostname: hostname = _(unknown)
 +self.window.get_widget(inspection-hostname).set_text(hostname)
 +product_name = self.vm.get_inspection_product_name()
 +if not product_name: product_name = _(unknown)
 +
 self.window.get_widget(inspection-product-name).set_text(product_name)
 +
 +# Applications (also inspection data)
 +apps = self.vm.get_inspection_applications()
 +

I'd just do

apps = self.vm.get_inspection_applications() or []

then you don't need to do any 'if apps', the 'for' loop will just be a nop.

 +if apps:
 +apps_list = self.window.get_widget(inspection-apps)
 +apps_model = apps_list.get_model()
 +apps_model.clear()
 +i = 0
 +for app in apps:
 +name = 
 +if app[app_name]:
 +name = app[app_name]
 +if app[app_display_name]:
 +name = app[app_display_name]
 +version = 
 +if app[app_version]:
 +version = app[app_version]
 +if app[app_release]:
 +version += - + app[app_release]
 +summary = 
 +if app[app_summary]:
 +summary = app[app_summary]
 +
 +apps_model.insert(i, [name, version, summary])
 +i += 1

You can drop 'i' and just do app_model.append([name, version, summary])

I like the idea though.

Thanks,
Cole

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