← Back to team overview

yahoo-eng-team team mailing list archive

[Bug 1590693] [NEW] libvirt's use of driver.get_instance_disk_info() is generally problematic

 

Public bug reported:

The nova.virt.driver 'interface' defines a get_instance_disk_info
method, which is called by compute manager to get disk info during live
migration to get the source hypervisor's internal representation of disk
info and pass it directly to the target hypervisor over rpc. To compute
manager this is an opaque blob of stuff which only the driver
understands, which is presumably why json was chosen. There are a couple
of problems with it.

This is a useful method within the libvirt driver, which uses it fairly
liberally. However, the method returns a json blob. Every use of it
internal to the libvirt driver first json encodes it in
get_instance_disk_info, then immediately decodes it again, which is
inefficient... except 2 uses of it in migrate_disk_and_power_off and
check_can_live_migrate_source, which don't decode it and assume it's a
dict. These are both broken, which presumably means something relating
to migration of volume-backed instances is broken. The libvirt driver
should not use this internally. We can have a wrapper method to do the
json encoding for compute manager, and internally use the unencoded data
data directly.

Secondly, we're passing an unversioned blob of data over rpc. We should
probably turn this data into a versioned object.

** Affects: nova
     Importance: Undecided
         Status: New


** Tags: low-hanging-fruit

** Description changed:

  The nova.virt.driver 'interface' defines a get_instance_disk_info
  method, which is called by compute manager to get disk info during live
  migration to get the source hypervisor's internal representation of disk
  info and pass it directly to the target hypervisor over rpc. To compute
  manager this is an opaque blob of stuff which only the driver
  understands, which is presumably why json was chosen. There are a couple
  of problems with it.
  
  This is a useful method within the libvirt driver, which uses it fairly
  liberally. However, the method returns a json blob. Every use of it
  internal to the libvirt driver first json encodes it in
  get_instance_disk_info, then immediately decodes it again, which is
- efficient. Except 2 uses of it in migrate_disk_and_power_off and
+ inefficient... except 2 uses of it in migrate_disk_and_power_off and
  check_can_live_migrate_source, which don't decode it and assume it's a
  dict. These are both broken, which presumably means something relating
  to migration of volume-backed instances is broken. The libvirt driver
  should not use this internally. We can have a wrapper method to do the
  json encoding for compute manager, and internally use the unencoded data
  data directly.
  
  Secondly, we're passing an unversioned blob of data over rpc. We should
  probably turn this data into a versioned object.

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/1590693

Title:
  libvirt's use of driver.get_instance_disk_info() is generally
  problematic

Status in OpenStack Compute (nova):
  New

Bug description:
  The nova.virt.driver 'interface' defines a get_instance_disk_info
  method, which is called by compute manager to get disk info during
  live migration to get the source hypervisor's internal representation
  of disk info and pass it directly to the target hypervisor over rpc.
  To compute manager this is an opaque blob of stuff which only the
  driver understands, which is presumably why json was chosen. There are
  a couple of problems with it.

  This is a useful method within the libvirt driver, which uses it
  fairly liberally. However, the method returns a json blob. Every use
  of it internal to the libvirt driver first json encodes it in
  get_instance_disk_info, then immediately decodes it again, which is
  inefficient... except 2 uses of it in migrate_disk_and_power_off and
  check_can_live_migrate_source, which don't decode it and assume it's a
  dict. These are both broken, which presumably means something relating
  to migration of volume-backed instances is broken. The libvirt driver
  should not use this internally. We can have a wrapper method to do the
  json encoding for compute manager, and internally use the unencoded
  data data directly.

  Secondly, we're passing an unversioned blob of data over rpc. We
  should probably turn this data into a versioned object.

To manage notifications about this bug go to:
https://bugs.launchpad.net/nova/+bug/1590693/+subscriptions


Follow ups