libvirt driver ignores 'disk_cachemodes' configuration setting

Bug #1727558 reported by melanie witt
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
High
Kashyap Chamarthy
Newton
Fix Committed
High
melanie witt
Ocata
Fix Committed
High
melanie witt
Pike
Fix Committed
High
melanie witt

Bug Description

The libvirt driver is ignoring the 'disk_cachemodes' configuration setting and is always setting "cache='none'" in the device xml.

For example, with a setting in nova.conf of "disk_cachemodes=network=writeback":

Expected result:

# virsh dumpxml <instance> | grep cache
        <driver name='qemu' type='raw' cache='writeback' discard='unmap'/>
        <driver name='qemu' type='raw' cache='writeback' discard='unmap'/>
        <driver name='qemu' type='raw' cache='writeback' discard='unmap'/>
        <driver name='qemu' type='raw' cache='writeback' discard='unmap'/>

Actual result:

# virsh dumpxml <instance> | grep cache
        <driver name='qemu' type='raw' cache='none' discard='unmap'/>
        <driver name='qemu' type='raw' cache='none' discard='unmap'/>
        <driver name='qemu' type='raw' cache='none' discard='unmap'/>
        <driver name='qemu' type='raw' cache='none' discard='unmap'/>

This is a regression in pike [1] that was also backported to ocata and newton.

[1] https://review.openstack.org/#/c/485752

Tags: libvirt
Changed in nova:
assignee: nobody → melanie witt (melwitt)
status: New → In Progress
Revision history for this message
melanie witt (melwitt) wrote :
description: updated
Changed in nova:
assignee: melanie witt (melwitt) → Matt Riedemann (mriedem)
Matt Riedemann (mriedem)
Changed in nova:
assignee: Matt Riedemann (mriedem) → Kashyap Chamarthy (kashyapc)
importance: Undecided → High
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/pike)

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/515538

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/ocata)

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/515540

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/newton)

Fix proposed to branch: stable/newton
Review: https://review.openstack.org/515543

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.openstack.org/514339
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=24e79bcbf7790d1f4fea2cbdf066599cc746c2dc
Submitter: Zuul
Branch: master

commit 24e79bcbf7790d1f4fea2cbdf066599cc746c2dc
Author: Kashyap Chamarthy <email address hidden>
Date: Mon Oct 23 16:27:01 2017 +0200

    libvirt: Don't disregard cache mode for instance boot disks

    One of the things this commit:

        commit 14c38ac0f253036da79f9d07aedf7dfd5778fde8
        Author: Kashyap Chamarthy <email address hidden>
        Date: Thu Jul 20 19:01:23 2017 +0200

            libvirt: Post-migration, set cache value for Cinder volume(s)

        [...]

    did was to supposedly remove "duplicate" calls to _set_cache_mode().

    But that came back to bite us.

    Now, while the Cinder volumes are taken care of w.r.t handling its cache
    value during migration, but the above referred commit (14c38ac) seemed
    to introduce a regression because it disregards the 'disk_cachemodes'
    Nova config parameter altogether for boot disks -- i.e. even though if
    a user set the cache mode to be 'writeback', it's ignored and
    instead 'none' is set unconditionally.

    Add the _set_cache_mode() calls back in _get_guest_storage_config().

    Co-Authored-By: melanie witt <email address hidden>

    Closes-Bug: #1727558

    Change-Id: I7370cc2942a6c8c51ab5355b50a9e5666cca042e

Changed in nova:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/newton)

Reviewed: https://review.openstack.org/515543
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=ca03fe87371d8c13fc2895f8ff3e7e1ca88cfe79
Submitter: Zuul
Branch: stable/newton

commit ca03fe87371d8c13fc2895f8ff3e7e1ca88cfe79
Author: Kashyap Chamarthy <email address hidden>
Date: Mon Oct 23 16:27:01 2017 +0200

    libvirt: Don't disregard cache mode for instance boot disks

    One of the things this commit:

        commit 14c38ac0f253036da79f9d07aedf7dfd5778fde8
        Author: Kashyap Chamarthy <email address hidden>
        Date: Thu Jul 20 19:01:23 2017 +0200

            libvirt: Post-migration, set cache value for Cinder volume(s)

        [...]

    did was to supposedly remove "duplicate" calls to _set_cache_mode().

    But that came back to bite us.

    Now, while the Cinder volumes are taken care of w.r.t handling its cache
    value during migration, but the above referred commit (14c38ac) seemed
    to introduce a regression because it disregards the 'disk_cachemodes'
    Nova config parameter altogether for boot disks -- i.e. even though if
    a user set the cache mode to be 'writeback', it's ignored and
    instead 'none' is set unconditionally.

    Add the _set_cache_mode() calls back in _get_guest_storage_config().

    Co-Authored-By: melanie witt <email address hidden>

    Closes-Bug: #1727558

     Conflicts:
     nova/virt/libvirt/driver.py

    NOTE(melwitt): The conflict is from a helper function
    _get_scsi_controller in ocata that doesn't exist in newton.

    Change-Id: I7370cc2942a6c8c51ab5355b50a9e5666cca042e
    (cherry picked from commit 24e79bcbf7790d1f4fea2cbdf066599cc746c2dc)
    (cherry picked from commit 60d6e87cac10ff1f95a028c6176e768214ec8b77)
    (cherry picked from commit fc10b54f25023d7e780b110928bda3a19e4c03f0)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/pike)

Reviewed: https://review.openstack.org/515538
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=60d6e87cac10ff1f95a028c6176e768214ec8b77
Submitter: Zuul
Branch: stable/pike

commit 60d6e87cac10ff1f95a028c6176e768214ec8b77
Author: Kashyap Chamarthy <email address hidden>
Date: Mon Oct 23 16:27:01 2017 +0200

    libvirt: Don't disregard cache mode for instance boot disks

    One of the things this commit:

        commit 14c38ac0f253036da79f9d07aedf7dfd5778fde8
        Author: Kashyap Chamarthy <email address hidden>
        Date: Thu Jul 20 19:01:23 2017 +0200

            libvirt: Post-migration, set cache value for Cinder volume(s)

        [...]

    did was to supposedly remove "duplicate" calls to _set_cache_mode().

    But that came back to bite us.

    Now, while the Cinder volumes are taken care of w.r.t handling its cache
    value during migration, but the above referred commit (14c38ac) seemed
    to introduce a regression because it disregards the 'disk_cachemodes'
    Nova config parameter altogether for boot disks -- i.e. even though if
    a user set the cache mode to be 'writeback', it's ignored and
    instead 'none' is set unconditionally.

    Add the _set_cache_mode() calls back in _get_guest_storage_config().

    Co-Authored-By: melanie witt <email address hidden>

    Closes-Bug: #1727558

    Change-Id: I7370cc2942a6c8c51ab5355b50a9e5666cca042e
    (cherry picked from commit 24e79bcbf7790d1f4fea2cbdf066599cc746c2dc)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/ocata)

Reviewed: https://review.openstack.org/515540
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=fc10b54f25023d7e780b110928bda3a19e4c03f0
Submitter: Zuul
Branch: stable/ocata

commit fc10b54f25023d7e780b110928bda3a19e4c03f0
Author: Kashyap Chamarthy <email address hidden>
Date: Mon Oct 23 16:27:01 2017 +0200

    libvirt: Don't disregard cache mode for instance boot disks

    One of the things this commit:

        commit 14c38ac0f253036da79f9d07aedf7dfd5778fde8
        Author: Kashyap Chamarthy <email address hidden>
        Date: Thu Jul 20 19:01:23 2017 +0200

            libvirt: Post-migration, set cache value for Cinder volume(s)

        [...]

    did was to supposedly remove "duplicate" calls to _set_cache_mode().

    But that came back to bite us.

    Now, while the Cinder volumes are taken care of w.r.t handling its cache
    value during migration, but the above referred commit (14c38ac) seemed
    to introduce a regression because it disregards the 'disk_cachemodes'
    Nova config parameter altogether for boot disks -- i.e. even though if
    a user set the cache mode to be 'writeback', it's ignored and
    instead 'none' is set unconditionally.

    Add the _set_cache_mode() calls back in _get_guest_storage_config().

    Co-Authored-By: melanie witt <email address hidden>

    Closes-Bug: #1727558

     Conflicts:
     nova/tests/unit/virt/libvirt/test_driver.py

    NOTE(melwitt): The conflict is from unit tests in pike that don't exist
    in ocata.

    Change-Id: I7370cc2942a6c8c51ab5355b50a9e5666cca042e
    (cherry picked from commit 24e79bcbf7790d1f4fea2cbdf066599cc746c2dc)
    (cherry picked from commit 60d6e87cac10ff1f95a028c6176e768214ec8b77)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 14.0.9

This issue was fixed in the openstack/nova 14.0.9 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 15.0.8

This issue was fixed in the openstack/nova 15.0.8 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 16.0.3

This issue was fixed in the openstack/nova 16.0.3 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 17.0.0.0b2

This issue was fixed in the openstack/nova 17.0.0.0b2 development milestone.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.