)]}'
{"specs/queens/approved/local-disk-serial-numbers.rst":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"57760ae3352fd9091df59db7adfe31f2078b0628","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"A user workload attaches an ephemeral disk and a persistent volume to an"},{"line_number":38,"context_line":"instance. The user wishes to configure both of these disks to mount by serial"},{"line_number":39,"context_line":"number."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"Proposed change"},{"line_number":42,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_c980968e","line":39,"updated":"2017-10-12 14:05:16.000000000","message":"I was thinking about this last night. To pick at your verbiage here, a user can attach a persistent volume, but they can\u0027t \"attach an ephemeral disk\". So I\u0027m trying to figure out how this would be used in the real world.\n\nI boot an instance, it comes up with the root, ephemeral, whatever disks. I attach a volume. The volume\u0027s serial number is predictable, so if I\u0027m ssh\u0027ing to the guest to do something with that volume, I could presumably find it via the serial number. However, I can\u0027t really know what the root disk\u0027s serial would be until I\u0027m booted, and after that, I\u0027m not sure it matters much? Are you expecting people to actually change their fstab to match the running state of the instance? That would break them for snapshots, among other things.\n\nMaybe we can expand on the details of the real use case here to make it clearer why we\u0027re doing this?","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e1fe412fde7635c982cab232ac3ac58fd4cfcbe8","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"A user workload attaches an ephemeral disk and a persistent volume to an"},{"line_number":38,"context_line":"instance. The user wishes to configure both of these disks to mount by serial"},{"line_number":39,"context_line":"number."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"Proposed change"},{"line_number":42,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_5cffaa33","line":39,"in_reply_to":"5f4e5783_a5ad7a89","updated":"2017-10-13 10:04:58.000000000","message":"So I\u0027ve gone back to the customer request and it\u0027s our old favourite: Windows activation checks. I\u0027ll add that. It\u0027s really a bugfix, tbh, as disks should just have serial numbers.\n\nHonestly, though, I\u0027m personally more interested in the dependant gains of:\n\n* Having a BDM uuid\n* Exposing local root BDM to driver through block_device_info\n\nThey\u0027re both much more interesting enablers.","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"0596536c0efd6caa649132f384272859a6e904c3","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"A user workload attaches an ephemeral disk and a persistent volume to an"},{"line_number":38,"context_line":"instance. The user wishes to configure both of these disks to mount by serial"},{"line_number":39,"context_line":"number."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"Proposed change"},{"line_number":42,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_d44b8e28","line":39,"in_reply_to":"5f4e5783_c980968e","updated":"2017-10-12 14:53:29.000000000","message":"As it happens this is a specific customer request. My use case is a best guess, tbh, as the RFE wasn\u0027t explicit but it makes sense to me.\n\nNotice that I close the loop on this below by tying in device role tagging (which I figured after writing this use case, so +1 to the process). The user can actually boot an instance with a tagged ephemeral disk, attach a volume, query metadata and identify the serial numbers of each.\n\nOf course, the real use case is probably something mundane like: our system management tools hate it when disks don\u0027t have serial numbers. I can write that too.","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2e9c63eb1fc9c1b1c4cbd702f1f1f031ff566184","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"A user workload attaches an ephemeral disk and a persistent volume to an"},{"line_number":38,"context_line":"instance. The user wishes to configure both of these disks to mount by serial"},{"line_number":39,"context_line":"number."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"Proposed change"},{"line_number":42,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_a5ad7a89","line":39,"in_reply_to":"5f4e5783_d44b8e28","updated":"2017-10-12 16:47:20.000000000","message":"Okay, I thought this was needed for some of the stable disk ordering, rescue, etc? If it\u0027s not and is just for \"there should be some serial number set\" then it seems rather unimportant (and maybe not even worth the data model change to keep it stable).\n\nI\u0027d be nice if the \"customer request\" could be turned into more detail. If it\u0027s really \"disks should have serial numbers\" then put that here so people can see how (un)important this is :)","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dcb77df9b655171bfe8c9e0e25d35534f842ba5f","unresolved":false,"context_lines":[{"line_number":51,"context_line":"DriverBlockDevice. This will additionally require us to expose the block device"},{"line_number":52,"context_line":"mapping of local root disks to virt drivers, which we do not currently do."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"If the local disk was given a device tag, the uuid will be exposed to the user"},{"line_number":55,"context_line":"via device metadata as for volumes."},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"This change will update the libvirt driver only, but will define an interface"},{"line_number":58,"context_line":"that other drivers should follow if they implement this feature."}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_572c4001","line":55,"range":{"start_line":54,"start_character":0,"end_line":55,"end_character":35},"updated":"2017-10-12 15:42:52.000000000","message":"I\u0027m missing where you make the connection here. We pass the tag through in the metadata/config drive but you\u0027d have to add something for the device UUID alongside the tag, yes?","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"60b867bdf24b3ab44cb1b8c8e03af56a71022285","unresolved":false,"context_lines":[{"line_number":51,"context_line":"DriverBlockDevice. This will additionally require us to expose the block device"},{"line_number":52,"context_line":"mapping of local root disks to virt drivers, which we do not currently do."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"If the local disk was given a device tag, the uuid will be exposed to the user"},{"line_number":55,"context_line":"via device metadata as for volumes."},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"This change will update the libvirt driver only, but will define an interface"},{"line_number":58,"context_line":"that other drivers should follow if they implement this feature."}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_37678428","line":55,"range":{"start_line":54,"start_character":0,"end_line":55,"end_character":35},"in_reply_to":"5f4e5783_572c4001","updated":"2017-10-12 15:50:22.000000000","message":"We only expose tagged devices in the device metadata, so you\u0027d have to explicitly tag your ephemeral disk at creation by using the bdm syntax. However, once it has a tag and we\u0027ve put the other bits in place, we can trivially expose it to the user this way.","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"57760ae3352fd9091df59db7adfe31f2078b0628","unresolved":false,"context_lines":[{"line_number":90,"context_line":"  such that it will not race with automatic uuid assignment to the same BDM"},{"line_number":91,"context_line":"  elsewhere."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"A data migration in a future release to be run after all python code has been"},{"line_number":94,"context_line":"updated should create uuids for all remaining block_device_mapping rows without"},{"line_number":95,"context_line":"one, and update the constraint to be not null. At this point, the python code"},{"line_number":96,"context_line":"handling a BDM without a uuid should be removed."}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_a98d5ad4","line":93,"range":{"start_line":93,"start_character":22,"end_line":93,"end_character":36},"updated":"2017-10-12 14:05:16.000000000","message":"s/this release/\n\nWhen we do these things, we include a data migration alongside it. Not all BDMs will be touched in normal operation, and for the fast-forward upgrade people, we have to provide a way to migrate the data without services running.","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0ed3f50d7b021f821ecec28c4b6af81e37d428ce","unresolved":false,"context_lines":[{"line_number":90,"context_line":"  such that it will not race with automatic uuid assignment to the same BDM"},{"line_number":91,"context_line":"  elsewhere."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"A data migration in a future release to be run after all python code has been"},{"line_number":94,"context_line":"updated should create uuids for all remaining block_device_mapping rows without"},{"line_number":95,"context_line":"one, and update the constraint to be not null. At this point, the python code"},{"line_number":96,"context_line":"handling a BDM without a uuid should be removed."}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_e9efad1b","line":93,"range":{"start_line":93,"start_character":22,"end_line":93,"end_character":36},"in_reply_to":"5f4e5783_1c4492b6","updated":"2017-10-13 14:51:25.000000000","message":"No, we have to do it _before_ we can enable the constraint. This is how we do all of these types of things. We open a hole, fill it, and then we can cover it over in the next release. Go look at every other case where we did an \"add uuid to $thing\" or \"move $thing from $placea to $placeb\" and the online migration came with the expansion.","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e1fe412fde7635c982cab232ac3ac58fd4cfcbe8","unresolved":false,"context_lines":[{"line_number":90,"context_line":"  such that it will not race with automatic uuid assignment to the same BDM"},{"line_number":91,"context_line":"  elsewhere."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"A data migration in a future release to be run after all python code has been"},{"line_number":94,"context_line":"updated should create uuids for all remaining block_device_mapping rows without"},{"line_number":95,"context_line":"one, and update the constraint to be not null. At this point, the python code"},{"line_number":96,"context_line":"handling a BDM without a uuid should be removed."}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_1c4492b6","line":93,"range":{"start_line":93,"start_character":22,"end_line":93,"end_character":36},"in_reply_to":"5f4e5783_85739e4d","updated":"2017-10-13 10:04:58.000000000","message":"My thinking here was that there\u0027s no point updating everything until we can enable the constraint, i.e. all code has been updated. Otherwise, when we come to enable the constraint later we\u0027re still going to have to do this again. You might as well not do it at all until you can do both. Once we\u0027re in a place where we can enable the constraint, we can also remove any code which assume\u0027s it might be empty, but again this is all dependent.\n\nIt\u0027s no great development effort to add this to the migration in the first instance so I\u0027m happy to add it. I just didn\u0027t think it was worth it as we\u0027ll still need to do it again.","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"0596536c0efd6caa649132f384272859a6e904c3","unresolved":false,"context_lines":[{"line_number":90,"context_line":"  such that it will not race with automatic uuid assignment to the same BDM"},{"line_number":91,"context_line":"  elsewhere."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"A data migration in a future release to be run after all python code has been"},{"line_number":94,"context_line":"updated should create uuids for all remaining block_device_mapping rows without"},{"line_number":95,"context_line":"one, and update the constraint to be not null. At this point, the python code"},{"line_number":96,"context_line":"handling a BDM without a uuid should be removed."}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_b49372a3","line":93,"range":{"start_line":93,"start_character":22,"end_line":93,"end_character":36},"in_reply_to":"5f4e5783_a98d5ad4","updated":"2017-10-12 14:53:29.000000000","message":"I\u0027d prefer to do it in the current release, but I didn\u0027t think it was possible. The issue I see is that you can\u0027t enable the constraint until all python code which touches the db has been updated, or it will start failing. Do we have an upgrade phase for that?","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dcb77df9b655171bfe8c9e0e25d35534f842ba5f","unresolved":false,"context_lines":[{"line_number":90,"context_line":"  such that it will not race with automatic uuid assignment to the same BDM"},{"line_number":91,"context_line":"  elsewhere."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"A data migration in a future release to be run after all python code has been"},{"line_number":94,"context_line":"updated should create uuids for all remaining block_device_mapping rows without"},{"line_number":95,"context_line":"one, and update the constraint to be not null. At this point, the python code"},{"line_number":96,"context_line":"handling a BDM without a uuid should be removed."}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_f7ea0caa","line":93,"range":{"start_line":93,"start_character":22,"end_line":93,"end_character":36},"in_reply_to":"5f4e5783_b49372a3","updated":"2017-10-12 15:42:52.000000000","message":"\u003e I\u0027d prefer to do it in the current release, but I didn\u0027t think it\n \u003e was possible. The issue I see is that you can\u0027t enable the\n \u003e constraint until all python code which touches the db has been\n \u003e updated, or it will start failing. Do we have an upgrade phase for\n \u003e that?\n\nWhen adding UUIDs to BDMs if they are older records, we\u0027d auto-generate the UUID on load of the older record if the UUID doesn\u0027t exist, in the BlockDeviceMapping object. That\u0027s how we handled this for Service and ComputeNode objects/records.","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"be179812289eab7f303aef847e41b049cb653df5","unresolved":false,"context_lines":[{"line_number":90,"context_line":"  such that it will not race with automatic uuid assignment to the same BDM"},{"line_number":91,"context_line":"  elsewhere."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"A data migration in a future release to be run after all python code has been"},{"line_number":94,"context_line":"updated should create uuids for all remaining block_device_mapping rows without"},{"line_number":95,"context_line":"one, and update the constraint to be not null. At this point, the python code"},{"line_number":96,"context_line":"handling a BDM without a uuid should be removed."}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_0fbe89d4","line":93,"range":{"start_line":93,"start_character":22,"end_line":93,"end_character":36},"in_reply_to":"5f4e5783_e9efad1b","updated":"2017-10-13 15:59:37.000000000","message":"For posterity: I spoke to Dan offline about this. I hadn\u0027t appreciated we already have a phase for this change, and that online migrations run in the current release, after code has updated. I\u0027ll update the spec to reflect this.\n\nThe online migration will populate all the values in the current release.\n\nIn the next release we can add a constraint and remove the code to handle null values. There\u0027s no need to re-run the online migration, because all code should already have been writing values for new objects at that point anyway.","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2e9c63eb1fc9c1b1c4cbd702f1f1f031ff566184","unresolved":false,"context_lines":[{"line_number":90,"context_line":"  such that it will not race with automatic uuid assignment to the same BDM"},{"line_number":91,"context_line":"  elsewhere."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"A data migration in a future release to be run after all python code has been"},{"line_number":94,"context_line":"updated should create uuids for all remaining block_device_mapping rows without"},{"line_number":95,"context_line":"one, and update the constraint to be not null. At this point, the python code"},{"line_number":96,"context_line":"handling a BDM without a uuid should be removed."}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_85739e4d","line":93,"range":{"start_line":93,"start_character":22,"end_line":93,"end_character":36},"in_reply_to":"5f4e5783_f7d44cbf","updated":"2017-10-12 16:47:20.000000000","message":"Right, but we still need the online migration for filling out anything that isn\u0027t touched. That goes into this release and thus we could introduce the constraint in the next release, having required everyone to fill out the values before moving.","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"60b867bdf24b3ab44cb1b8c8e03af56a71022285","unresolved":false,"context_lines":[{"line_number":90,"context_line":"  such that it will not race with automatic uuid assignment to the same BDM"},{"line_number":91,"context_line":"  elsewhere."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"A data migration in a future release to be run after all python code has been"},{"line_number":94,"context_line":"updated should create uuids for all remaining block_device_mapping rows without"},{"line_number":95,"context_line":"one, and update the constraint to be not null. At this point, the python code"},{"line_number":96,"context_line":"handling a BDM without a uuid should be removed."}],"source_content_type":"text/x-rst","patch_set":1,"id":"5f4e5783_f7d44cbf","line":93,"range":{"start_line":93,"start_character":22,"end_line":93,"end_character":36},"in_reply_to":"5f4e5783_f7ea0caa","updated":"2017-10-12 15:50:22.000000000","message":"Yep, that\u0027s what I describe above. I was envisioning here a way to eventually un-cruft the object by asserting in the DB that the uuid exists. This step isn\u0027t strictly necessary at all, but I think it\u0027s a good goal as it defines an endpoint to the transition phase.","commit_id":"0280f55c2677d028feff9edb871de174ad79fcfb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"193933b211e9ae9d9ec55823936ca06c0114b977","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"Windows uses a number of metrics to determine if reactivation is required,"},{"line_number":42,"context_line":"including disk serial numbers. Having stable serial numbers reduces the"},{"line_number":43,"context_line":"probability of requiring a reactivation."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"Proposed change"}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_6f23bdbe","line":43,"updated":"2017-10-13 16:29:13.000000000","message":"This isn\u0027t really written as a use-case and as such probably hides some of the detail here. Consider this:\n\n\"As a user running windows guests, I need to avoid having activation issues due to a lack of a stable disk serial number.\"\n\nand\n\n\"As an admin, I need to be able to migrate windows guests to address maintenance needs in the datacenter without risking de-activating customer windows instances because disk serial numbers are not stable.\"","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f5c18c83054d5024305bfac6f4e1e7cd0204a36c","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"Windows uses a number of metrics to determine if reactivation is required,"},{"line_number":42,"context_line":"including disk serial numbers. Having stable serial numbers reduces the"},{"line_number":43,"context_line":"probability of requiring a reactivation."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"Proposed change"}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_6a5e0b45","line":43,"in_reply_to":"5f4e5783_6f23bdbe","updated":"2017-10-13 17:17:45.000000000","message":"Yep, those read better. Will update.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"193933b211e9ae9d9ec55823936ca06c0114b977","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Serial numbers will be exposed to virt drivers by adding a serial field to"},{"line_number":56,"context_line":"DriverBlockDevice. This will additionally require us to expose the block device"},{"line_number":57,"context_line":"mapping of local root disks to virt drivers, which we do not currently do."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If the local disk was given a device tag, the uuid will be exposed to the user"},{"line_number":60,"context_line":"via device metadata as for volumes."}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_ea2e7b90","line":57,"updated":"2017-10-13 16:29:13.000000000","message":"Hmm, I had skipped over this change in the first read of this. I assume you\u0027re going to fix (or coordinate fixes for) all the other drivers for this right? Since this is affecting guest-visible \"Nova ABI\" I don\u0027t think it\u0027s reasonable to just do this for libvirt. Users expecting to boot instances on a cloud and get this behavior don\u0027t know or care what hypervisor is in use, and will find it confusing if the new behavior doesn\u0027t happen consistently, IMHO.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cbd9419a17efdf582f87c464698e359b19a3910a","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Serial numbers will be exposed to virt drivers by adding a serial field to"},{"line_number":56,"context_line":"DriverBlockDevice. This will additionally require us to expose the block device"},{"line_number":57,"context_line":"mapping of local root disks to virt drivers, which we do not currently do."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If the local disk was given a device tag, the uuid will be exposed to the user"},{"line_number":60,"context_line":"via device metadata as for volumes."}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_748711b7","line":57,"in_reply_to":"5f4e5783_8a7827c4","updated":"2017-10-13 18:56:51.000000000","message":"Okay, I was reading this as \"we don\u0027t pass the virt driver a BDM for the root device today, but we will start doing so.\" Presumably you really just meant we didn\u0027t distinguish the root device or something?\n\nI want to make sure we don\u0027t have a virt driver that assumes any BDM it is passed to be a volume, and tries to attach it as such.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f5c18c83054d5024305bfac6f4e1e7cd0204a36c","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Serial numbers will be exposed to virt drivers by adding a serial field to"},{"line_number":56,"context_line":"DriverBlockDevice. This will additionally require us to expose the block device"},{"line_number":57,"context_line":"mapping of local root disks to virt drivers, which we do not currently do."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If the local disk was given a device tag, the uuid will be exposed to the user"},{"line_number":60,"context_line":"via device metadata as for volumes."}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_8a7827c4","line":57,"in_reply_to":"5f4e5783_ea2e7b90","updated":"2017-10-13 17:17:45.000000000","message":"I wasn\u0027t going to trouble the spec with this level of detail, but in essence I\u0027m going to add \u0027root\u0027 to block_device_info. Drivers which don\u0027t expect \u0027root\u0027 to be there won\u0027t use it, so won\u0027t be affected by it.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"193933b211e9ae9d9ec55823936ca06c0114b977","unresolved":false,"context_lines":[{"line_number":57,"context_line":"mapping of local root disks to virt drivers, which we do not currently do."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If the local disk was given a device tag, the uuid will be exposed to the user"},{"line_number":60,"context_line":"via device metadata as for volumes."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"This change will update the libvirt driver only, but will define an interface"},{"line_number":63,"context_line":"that other drivers should follow if they implement this feature."}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_0a4f976b","line":60,"updated":"2017-10-13 16:29:13.000000000","message":"Won\u0027t we expose the disk and uuid regardless of if they provide a tag? Does the device metadata only include tagged devices or everything? I had assumed the latter.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f5c18c83054d5024305bfac6f4e1e7cd0204a36c","unresolved":false,"context_lines":[{"line_number":57,"context_line":"mapping of local root disks to virt drivers, which we do not currently do."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If the local disk was given a device tag, the uuid will be exposed to the user"},{"line_number":60,"context_line":"via device metadata as for volumes."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"This change will update the libvirt driver only, but will define an interface"},{"line_number":63,"context_line":"that other drivers should follow if they implement this feature."}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_1e78fc8d","line":60,"in_reply_to":"5f4e5783_0a4f976b","updated":"2017-10-13 17:17:45.000000000","message":"My reading of the code is that we only expose tagged devices:\n\n  https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7926-L7928\n\nPresumably there\u0027s not much point exposing an untagged disk, as the user has nothing to correlate it against?","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cbd9419a17efdf582f87c464698e359b19a3910a","unresolved":false,"context_lines":[{"line_number":57,"context_line":"mapping of local root disks to virt drivers, which we do not currently do."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"If the local disk was given a device tag, the uuid will be exposed to the user"},{"line_number":60,"context_line":"via device metadata as for volumes."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"This change will update the libvirt driver only, but will define an interface"},{"line_number":63,"context_line":"that other drivers should follow if they implement this feature."}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_547e55af","line":60,"in_reply_to":"5f4e5783_1e78fc8d","updated":"2017-10-13 18:56:51.000000000","message":"Well, from the tag perspective, obviously. However, the information about how things are attached (driver, bus, slot, etc) is still relevant.\n\nRegardless, if this isn\u0027t a change then it\u0027s not a problem.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"193933b211e9ae9d9ec55823936ca06c0114b977","unresolved":false,"context_lines":[{"line_number":60,"context_line":"via device metadata as for volumes."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"This change will update the libvirt driver only, but will define an interface"},{"line_number":63,"context_line":"that other drivers should follow if they implement this feature."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"Alternatives"},{"line_number":66,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_0a78b78d","line":63,"updated":"2017-10-13 16:29:13.000000000","message":"Won\u0027t this break them? If they get only volume BDMs right now won\u0027t they try to process those and potentially do bad things? I think you need to be on the hook to fix those or coordinate owners providing fixes.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f5c18c83054d5024305bfac6f4e1e7cd0204a36c","unresolved":false,"context_lines":[{"line_number":60,"context_line":"via device metadata as for volumes."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"This change will update the libvirt driver only, but will define an interface"},{"line_number":63,"context_line":"that other drivers should follow if they implement this feature."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"Alternatives"},{"line_number":66,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_be432813","line":63,"in_reply_to":"5f4e5783_0a78b78d","updated":"2017-10-13 17:17:45.000000000","message":"block_device_info is a dict containing DriverBlockDevice objects (well, subclasses). From memory (but pretty sure it\u0027s about right):\n\n{\n  \u0027root_device\u0027: \u0027/dev/foo\u0027,\n  \u0027ephemerals\u0027: [DBD(), ...]\n  \u0027swap\u0027: DBD() or none\n  \u0027block_device_mappings\u0027: [DBD(), ...]\n}\n\nWhere \u0027block_device_mappings\u0027 means \u0027volumes\u0027, but is just a differently-filtered list (sigh).\n\nI\u0027m just going to add \u0027root\u0027 containing a DBD() for the root disk. I don\u0027t expect this to break drivers which aren\u0027t looking for it.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cbd9419a17efdf582f87c464698e359b19a3910a","unresolved":false,"context_lines":[{"line_number":60,"context_line":"via device metadata as for volumes."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"This change will update the libvirt driver only, but will define an interface"},{"line_number":63,"context_line":"that other drivers should follow if they implement this feature."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"Alternatives"},{"line_number":66,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_b43d89de","line":63,"in_reply_to":"5f4e5783_be432813","updated":"2017-10-13 18:56:51.000000000","message":"Okay, but like I was saying above, if I go through block_device_mappings and expect each of those to be a volume I need to attach, I won\u0027t know to look for/ignore the root one, right? At least a survey of the other drivers to make sure this won\u0027t break them (or identify what needs to be done) would be good I think.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"193933b211e9ae9d9ec55823936ca06c0114b977","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"This change will update the libvirt driver only, but will define an interface"},{"line_number":63,"context_line":"that other drivers should follow if they implement this feature."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"Alternatives"},{"line_number":66,"context_line":"------------"},{"line_number":67,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_2a4ff334","line":64,"updated":"2017-10-13 16:29:13.000000000","message":"Can you add something in here about ironic? Presumably we can\u0027t override the disk serial that they see, and we don\u0027t want to expose the uuid in metadata when it doesn\u0027t match what the disk actually reports.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f5c18c83054d5024305bfac6f4e1e7cd0204a36c","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"This change will update the libvirt driver only, but will define an interface"},{"line_number":63,"context_line":"that other drivers should follow if they implement this feature."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"Alternatives"},{"line_number":66,"context_line":"------------"},{"line_number":67,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_bedfa838","line":64,"in_reply_to":"5f4e5783_2a4ff334","updated":"2017-10-13 17:17:45.000000000","message":"Hmm, hadn\u0027t considered ironic... I wonder if this needs to be driver-populated after all. There\u0027s also the possibility that a virt driver might have default serial number behaviour that we would be changing. This might require a rethink.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cbd9419a17efdf582f87c464698e359b19a3910a","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"This change will update the libvirt driver only, but will define an interface"},{"line_number":63,"context_line":"that other drivers should follow if they implement this feature."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"Alternatives"},{"line_number":66,"context_line":"------------"},{"line_number":67,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_940f2d36","line":64,"in_reply_to":"5f4e5783_bedfa838","updated":"2017-10-13 18:56:51.000000000","message":"Yeah, I can totally see VMDK embedding the serial number in some way that makes it hard to remain consistent here.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"193933b211e9ae9d9ec55823936ca06c0114b977","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"Python code will be updated such that:"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"* new BDM objects are never created without a uuid."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"* BDM objects without a uuid are assigned one automatically on either read or"},{"line_number":94,"context_line":"  write, transparent to the caller. Automatic uuid assignment will be written"}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_6aadebe7","line":91,"updated":"2017-10-13 16:29:13.000000000","message":"I read this several times before realizing the double negative. It would be clearer if you changed this to:\n\n New BDM objects will always be created with a uuid","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f5c18c83054d5024305bfac6f4e1e7cd0204a36c","unresolved":false,"context_lines":[{"line_number":88,"context_line":""},{"line_number":89,"context_line":"Python code will be updated such that:"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"* new BDM objects are never created without a uuid."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"* BDM objects without a uuid are assigned one automatically on either read or"},{"line_number":94,"context_line":"  write, transparent to the caller. Automatic uuid assignment will be written"}],"source_content_type":"text/x-rst","patch_set":2,"id":"5f4e5783_beb8c8e4","line":91,"in_reply_to":"5f4e5783_6aadebe7","updated":"2017-10-13 17:17:45.000000000","message":"Indeed.","commit_id":"37fe41342918ec3df01bba655e5a90336c445952"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f65817337c3018c9803ece410ca27bb5bf5a5a8c","unresolved":false,"context_lines":[{"line_number":178,"context_line":"* Expose local root disks to virt drivers via block_device_info."},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"* Add serial field to DriverBlockDevice with the existing behaviour for"},{"line_number":181,"context_line":"  volumes, and the new bdm uuid for local disks."},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"* Update libvirt driver to use DriverBlockDevice.serial for all disks."},{"line_number":184,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3f4b6375_595ca84a","line":181,"updated":"2017-10-19 14:33:28.000000000","message":"In some ways its a shame they are different, but this seems the best way forward now.","commit_id":"dc94b61582b7ecd7434c7e5ac5ddd94329ab42ee"}]}
