Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-05
19:51:15 dansmith I think the thing that led me there, aside from artom's comment,
19:51:34 dansmith was that you're saying "if we don't have access to the API DB" which can't happen without nothing else working, if it's just in the api
19:52:00 dansmith and the log message to that effect
19:52:43 dansmith the other change you reference was to handle calling code that could be either api-level or inside the cell
19:52:59 dansmith but if this is only ever at the API, then I don't think there's any need to catch that situation
19:53:05 dansmith and if you do,
19:53:10 dansmith it's not info-level, it's error-level,
19:53:20 dansmith along with all the other tracebacks the api would be throwing if you even got that far :)
19:56:43 mriedem ok, like i said, the CantStartEngineError thing came up in denver i think, but it's not written down so i can't remember the details for sure - i don't need it, and agree it shouldn't happen - and if it does, we're more f'ed than just this, so i'm happy to remove that in a follow up when i fix that comment in the test
19:57:11 mriedem or, if i'm backporting, just nack it and i can remove that now
19:57:28 dansmith ack, commenting
19:58:55 dansmith mriedem: done. left my +2, you can decide to fix or fup
19:59:07 dansmith for backport, I'd think it's worth fixing before it lands, since it's like 24 hours away :)
20:04:12 mriedem to be clear,
20:04:14 mriedem "So, if you're going to catch it and log, I'd say do so at error and then re-raise the exception like all the other API code would."
20:04:18 mriedem you're OK with just removing that catch right?
20:05:16 dansmith yes, better to just remove it, imho
20:05:27 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Find instance in another cell during floating IP re-association https://review.opendev.org/656594
20:06:38 dansmith mriedem: you doing it now? I'll stand by to fast approve if so
20:07:35 mriedem no, still going through this big numa lm patch
20:07:44 mriedem just got through the commit message...
20:08:47 openstackgerrit Eric Fried proposed openstack/nova master: Nova object changes for forbidden aggregates request filter https://review.opendev.org/671072
20:08:48 openstackgerrit Eric Fried proposed openstack/nova master: DB API changes to get non-matching aggregates from metadata https://review.opendev.org/671074
20:08:48 openstackgerrit Eric Fried proposed openstack/nova master: Add a new request filter to isolate aggregates https://review.opendev.org/671075
20:08:49 openstackgerrit Eric Fried proposed openstack/nova master: Docs for isolated aggregates request filter https://review.opendev.org/667952
20:09:11 efried dansmith: gotta go on next chauffeur run, but I think that's what we were shooting for ^
20:10:16 efried basically half of the .py changes needed to be in the first patch, the other half in the third.
20:11:12 efried guess I should have done that field rename while I was in there, sec...
20:15:53 openstackgerrit Eric Fried proposed openstack/nova master: Nova object changes for forbidden aggregates request filter https://review.opendev.org/671072
20:15:53 openstackgerrit Eric Fried proposed openstack/nova master: DB API changes to get non-matching aggregates from metadata https://review.opendev.org/671074
20:15:58 openstackgerrit Eric Fried proposed openstack/nova master: Add a new request filter to isolate aggregates https://review.opendev.org/671075
20:15:58 openstackgerrit Eric Fried proposed openstack/nova master: Docs for isolated aggregates request filter https://review.opendev.org/667952
20:16:13 mriedem totally unrelated to the numa live migration stuff, but can this fail the 'claim' for sriov pci devices on the dest host? https://github.com/openstack/nova/blob/f7f5e1846c7b19aa05817df7f3c4345819db413f/nova/compute/manager.py#L6491
20:16:16 mriedem and if so, how does it fail?
20:17:07 mriedem because there is a very specific set of exceptions that will trigger a reschedule to an alternate host https://github.com/openstack/nova/blob/f7f5e1846c7b19aa05817df7f3c4345819db413f/nova/conductor/tasks/live_migrate.py#L482
20:17:23 mriedem so if that stuff doesn't result in MigrationPreCheckError we'll fail the live migratoin b/c the first host claim failed
20:17:26 mriedem sean-k-mooney: ^
20:18:41 openstackgerrit Dan Smith proposed openstack/nova master: Find instance in another cell during floating IP re-association https://review.opendev.org/656594
20:18:44 mriedem trying to follow that pci claim code is going down a rabbit hole
20:18:51 dansmith mriedem: ^
20:20:50 mriedem want to hit that test comment while you're updating?
20:43:26 openstackgerrit Merged openstack/nova master: neutron: refactor nw info cache refresh out of associate_floating_ip https://review.opendev.org/678300
20:56:31 mriedem oooo https://github.com/openstack/nova/blob/f7f5e1846c7b19aa05817df7f3c4345819db413f/nova/scheduler/filter_scheduler.py#L173-L187 - if it weren't for USES_ALLOCATION_CANDIDATES we could burn all of that queens compat code out of the filter scheduler
20:57:51 mriedem artom: dansmith: ok comments in the big one https://review.opendev.org/#/c/634606/75
21:05:40 openstackgerrit Matt Riedemann proposed openstack/nova master: Find instance in another cell during floating IP re-association https://review.opendev.org/656594
21:05:45 dansmith mriedem: cool thanks
21:05:58 mriedem ok i think that multi-cell floating ip thing is ready
21:06:33 artom mriedem, thanks! It's supper time now, will work a bit after, then gym, so tomorrow morning
21:06:43 artom Haven't been to gym in, like, a week, not cool
21:08:39 openstackgerrit Merged openstack/nova master: Trap and log errors from _update_inst_info_cache_for_disassociated_fip https://review.opendev.org/678301
21:13:32 mriedem gerrit is down anyway
21:21:18 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove old comments about caching scheduler compat https://review.opendev.org/680521
21:36:11 openstackgerrit Eric Fried proposed openstack/nova master: Add a new request filter to isolate aggregates https://review.opendev.org/671075
21:36:11 openstackgerrit Eric Fried proposed openstack/nova master: Docs for isolated aggregates request filter https://review.opendev.org/667952
21:37:13 mriedem alex_xu: can you take a look at the api change to attach a volume (and list/show vol attachments) with delete_on_termination? https://review.opendev.org/#/c/673133/ - it's probably going to be 2.79; i'm +2 on it
21:42:59 openstackgerrit Adam Spiers proposed openstack/nova master: Ensure non-q35 machine type is not used when booting with SEV https://review.opendev.org/680065
21:43:32 openstackgerrit Adam Spiers proposed openstack/nova master: Extract fake KVM guest fixture for reuse https://review.opendev.org/680526
21:44:47 openstackgerrit Adam Spiers proposed openstack/nova master: Move get_machine_type() test to test_utils.py https://review.opendev.org/680527
21:57:03 efried_afk dansmith: vpmem json blob => hyp-specific ovo (https://review.opendev.org/#/c/678448/), you said to follow the example of LiveMigrateData.
21:57:03 efried_afk So I think that means they should make a base class (e.g. ResourceMetadata) and make LibvirtPMEMDevice (https://review.opendev.org/#/c/678453/19/nova/objects/resource.py) a subclass thereof.
21:57:03 efried_afk I can't see where LiveMigrateData is a field in another ovo, though; is it kosher to make Resource.metadata of type ResourceMetadata even though IRL it will always be a *subclass*?
21:57:39 openstackgerrit Matt Riedemann proposed openstack/nova master: Rename Claims resources to compute_node https://review.opendev.org/679470
21:58:13 dansmith yeah, although you might have to register the base to make that work, which I don't think was required for the MD case because we always sent the child and not the parent, IIRC, but it's been a whiel
21:58:33 efried_afk what does "register the base" mean?
21:59:40 mriedem @obj_base.NovaObjectRegistry.register
22:00:10 mriedem note that the base LiveMigrateData is not registered
22:00:11 mriedem https://github.com/openstack/nova/blob/master/nova/objects/migrate_data.py#L112
22:00:16 mriedem b/c everything uses the subclasses
22:00:25 dansmith right that ^
22:00:48 dansmith efried: ovo requires a shared registry of the schemas on either side to ensure proper versioning
22:00:50 efried everything will use the subclass in this case too... unless you mean that the Resource object will "use" the base class?
22:00:55 mriedem so i can't do something like this in my driver: migrate_data = objects.LiveMigrateData(all_my_cool_shenanigans={...})
22:01:07 dansmith efried: the difference is the inclusion in a field, which has to specify an object in the registry, IIRC
22:01:14 dansmith which is why it's by name and not by reference
22:01:17 efried okay, cool.
22:03:58 efried dansmith, mriedem: Did I say it right? https://review.opendev.org/#/c/678448/15/nova/objects/resource.py@36
22:05:01 mriedem i don't know that the chinese are going to understand the south park underpants gnomes reference
22:05:20 efried I've never actually even seen that thing
22:05:33 efried indirect meme reference culture
22:05:52 dansmith efried: commented
22:05:55 efried I could find it if I wanted
22:06:33 efried dansmith: thanks. I think nullable because not all ResourceZ are going to have metadata.
22:06:41 mriedem then don't set the field
22:06:44 dansmith efried: yeah, that's the wrong answer
22:06:47 efried ah
22:07:25 efried unset vs value of None vs object with no fields initialized vs object with fields set to None vs .....
22:07:26 dansmith I thought I already commented about this in this patch, but maybe I was overwhelmed by the larger sin of using a string here and it didn't come through
22:07:42 mriedem i need to make dan a shirt that has nullable=True on some object field with the circle bar banned icon on it
22:07:49 dansmith I'd wear it
22:08:17 dansmith efried: I suspect the right thing there is to always have a metadata object in that field (hence never None), with either fields set appropriately empty or whatever else,
22:08:26 dansmith but it's hard to tell what the right thing is because it's not defined there
22:08:43 dansmith that's probably why I didn't comment, because I don't know WHAT that is, other than a dumping ground for blobs
22:10:03 mriedem it's this thing i think https://review.opendev.org/#/c/678453/19/nova/objects/resource.py
22:10:03 efried it's for hypervisor-specific, resource-specific metadata for a Resource. And not all ResourceZ are going to have any. It would seem like a lot of work to a) make sure to set the value b) to an empty object, meaning c) you have to make sure the object type can be "empty", whatever that means, etc.
22:10:12 mriedem used here https://review.opendev.org/#/c/678454/19/nova/virt/libvirt/driver.py@6934
22:10:33 mriedem efried: not empty, just don't set the field
22:10:44 efried so does "nullable=False" just mean you're not allowed to set it to None?
22:10:48 mriedem correct
22:10:53 efried and that's the default behavior.
22:11:01 mriedem yeah

Earlier   Later