Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-05
14:49:41 efried dansmith, tinwood: I looked at the in-tree usages of get_instance_diagnostics, and they're all using a string to set that field; none of them are using the actual enum class. So changing that to a string field would require no changes *in tree*. Out of tree, I suppose it would be harmless if we kept the enum around (deprecated) and let folks know to transition to using a plain string.
14:50:13 tinwood efried, I tried that, but it errored out as the field checks the enum for the string.
14:50:30 tinwood That's how I found I needed to update the enum.
14:50:53 efried tinwood: You tried just using a string in your oot get_instance_diagnostics impl?
14:52:54 openstackgerrit Matt Riedemann proposed openstack/nova master: Drop the integrated-gate (py27) template https://review.openstack.org/634949
14:53:07 tinwood efried, yes, I wouldn't have changed nova except it wouldn't accept 'lxd'. The equivalent change in nova-lxd needing it is a string: https://review.openstack.org/#/c/633270/3/nova/virt/lxd/driver.py
14:53:38 tinwood efried, line 1175
14:53:44 efried Yeah, so that's what I'm saying. Instead of augmenting the enum every time some OOT (or in-tree for that matter) driver needs to implement get_instance_diagnostics, let's just change the field in the Diagnostics object to a StringField. We can deprecate the HypervisorDriver enum (or not, I guess it doesn't really matter if drivers want to use it to get their string). That way, any impl - in tree or out - can use any string they l
14:53:44 efried without having to dork with this enum.
14:54:33 tinwood efried, ah, I see - well, it would help me ... :)
14:54:44 efried I mean, I get the value in having a discrete set of values allowed in that field, but I don't think it's probably worth the hassle in this case.
14:55:05 efried dansmith: would you be supportive of such a change?
14:55:47 dansmith efried: we should ask whoever added that
14:55:56 dansmith but this falls into the realm of my previous meh on the topic
14:56:09 efried sergei nitikin
14:56:18 efried nikitin
14:56:25 efried blueprint restore-vm-diagnostics
14:57:48 cdent mriedem: to avoid a pointless "woot, yeah" response to your recent email, I say "woot, yeah" here.
14:57:50 cdent woot, yeah
14:58:49 efried tinwood: ^
14:59:06 tinwood efried, taking a look
14:59:14 mriedem cdent: woot acknowledged
14:59:46 efried The spec doesn't say it's an enum. It says it's a string. The enum-ness must have been decided during impl time.
15:00:44 tinwood efried, indeed. But I can't merge my nova-lxd change as nova either needs a change to the enum-ness or adding the lxd bit. I could take a look at removing the enumness if that would help?
15:00:56 tinwood efried, which would you prefer?
15:02:03 efried tinwood: Yes, clearly you need a change to nova, I get that. I also get dansmith's position of not wanting to enumerate OOT drivers in an in-tree object. I think changing to a string field is a good compromise. Can you dig it?
15:02:20 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove PLACEMENT_DB_ENABLED from nova-next job config https://review.openstack.org/634953
15:02:22 tinwood efried, sure, happy to take a crack at that.
15:02:22 efried your code will work the same either way
15:02:31 efried okay. I'll leave a comment on your review.
15:02:46 efried tinwood: you can use the same change set, so we can keep track of this thread of discussion.
15:03:11 tinwood efried, okay, will do.
15:09:06 efried tinwood: wrt getting rid of HypervisorDriver[Field], my opinion is that we should just kill it, with a warning sent out to the ML in case downstream/oot code happens to be using it (which seems like a really low probability). Would you like to craft that patch and/or ML post as well?
15:10:36 tinwood Sure, I can do the patch. The ML patch; yes, I can probably draft something.
15:12:54 efried tinwood: Thanks!
15:13:00 tinwood np :)
15:26:35 openstackgerrit Stephen Finucane proposed openstack/nova master: API: Remove evacuate/live-migrate 'force' parameter https://review.openstack.org/634600
15:31:31 sean-k-mooney efried: which hypervior driver filed
15:31:52 efried sean-k-mooney: See https://review.openstack.org/#/c/633279/
15:33:00 sean-k-mooney efried: rather then kill it why not make it an string field
15:34:04 efried sean-k-mooney: That's the plan: make the .driver field a StringField. But that's the only place HypervisorDriver[Field] is used, so we can get rid of that object.
15:35:11 sean-k-mooney oh ok
15:35:38 sean-k-mooney efried: for compatiablity i think we woudl need to rename it too
15:36:09 efried sean-k-mooney: I don't think so. The data type per se isn't changing.
15:36:15 sean-k-mooney efried: as in i dont think we are allowed to chagne types of fields in OVO
15:36:30 efried sean-k-mooney: All existing in-tree consumers use a string to assign the field
15:37:27 sean-k-mooney that is not my concern im wonderign how this will work during an upgrade
15:38:12 sean-k-mooney e.g. if you had an old api node and a new compute
15:38:48 sean-k-mooney i guess that shoudl not be and issue as you should upgrade your api and condocutor before the computes
15:38:59 sean-k-mooney at least for majour upgrades
15:39:00 efried adding an enum value doesn't make that better though, right?
15:39:08 efried still have the same problem
15:39:42 efried if you receive an object and it has a value you don't recognize in that field, you're borked either way.
15:40:00 efried but at least making it a string field will avoid that problem in the future.
15:40:02 sean-k-mooney well you would need to handel it in the object make compatioble function
15:40:07 sean-k-mooney when you are downleveling it
15:40:55 sean-k-mooney ya im just wondering if we shoudl rename it to driver_name when we make it a sting
15:42:47 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Fix legacy-grenade-dsvm-neutron-multinode-live-migration https://review.openstack.org/634962
15:42:50 openstackgerrit Balazs Gibizer proposed openstack/nova master: Ensure that bandwidth and VF are from the same PF https://review.openstack.org/623543
15:42:54 efried I'm not an ovo expert by any means. If that's necessary, so be it.
15:43:59 sean-k-mooney efried: well i usuall check with dansmith for these kindo fo things. but in the past he has alswas asked if i am changing form a dict to an ovo for example that the filed be renamed.
15:44:09 sean-k-mooney enum to string is slightly differnet
15:44:17 sean-k-mooney but technically its the same
15:44:27 sean-k-mooney you are still changeing datatypes
15:45:31 efried Guess it depends how ovo is treating the enum. In python the enum value is just a string.
15:46:08 sean-k-mooney yes the underlying python type is a sting in either case
15:46:14 efried other than checking against the set of valid values, there *shouldn't* be any extra significance tied to the fact that it's an enum.
15:46:33 sean-k-mooney an enum field is the same a string field with the choices attibute set
15:47:05 gibi_off melwitt: hi! I'm still not 100% back but I can chair the nova meeting on Thursday as normal.
15:48:19 openstackgerrit Stephen Finucane proposed openstack/nova master: API: Remove evacuate/live-migrate 'force' parameter https://review.openstack.org/634600
15:48:56 sean-k-mooney efried: it just field like casting to void*. the data in memory is still the same but how you use it just chaged. its not somting i would block it on but i would still check with jaypipes or dansmith personally
15:49:11 sean-k-mooney *feels like
15:49:47 dansmith sean-k-mooney: yep, agree, and since this is for something out of tree, it's not really worth doing something like that to make it work, imho
15:51:41 sean-k-mooney dansmith: so to be clear are you +/- on changing to string and if we do change to string would you also want the field name change form driver to driver_name?
15:53:23 dansmith sean-k-mooney: char *response = "meh"; int vote = -1 *(int*)(void*)response;
15:55:12 dansmith hrm, my C is rusty
15:57:55 sean-k-mooney i belive that woudl set vote to -1 * the address of responce.
15:58:42 dansmith I meant to have another * in there, so it tried to set vote to -1 * the intlike value of meh, which would be negative something large :)
15:59:26 sean-k-mooney yes i assumed that was what you were going for. poters are always fun...
16:00:36 sean-k-mooney *pointers
16:07:38 mriedem need another stable core for this https://review.openstack.org/#/c/627011/
16:07:57 mriedem you guys are still talking about this out of tree enum thing? :)
16:11:29 openstackgerrit Rodolfo Alonso Hernandez proposed openstack/os-vif master: Retrieve the table schema using ovsdb-client command https://review.openstack.org/634967
16:12:53 sean-k-mooney mriedem: i think we had finished but yes
16:13:52 openstackgerrit Andrey Volkov proposed openstack/nova master: Check hosts have no instances for AZ rename https://review.openstack.org/509206
16:14:20 sean-k-mooney mriedem: by the way i agree with your ml post regardign changing the jobs. when i get my third party ci triggering of upstream it would be good to get your input on what you would like it to test
16:15:07 mriedem ok
16:17:33 openstackgerrit sean mooney proposed openstack/nova master: Libvirt: do not set mac when unplugging macvtap vf https://review.openstack.org/624842
16:17:34 openstackgerrit sean mooney proposed openstack/nova master: Add free for claimed, allocated devices https://review.openstack.org/616120
16:17:34 openstackgerrit sean mooney proposed openstack/nova master: Allow per-port modification of vnic_type and profile https://review.openstack.org/607365
16:17:35 openstackgerrit sean mooney proposed openstack/nova master: Add get_instance_pci_request_from_vif https://review.openstack.org/619929
16:17:35 openstackgerrit sean mooney proposed openstack/nova master: SR-IOV Live migration indirect port support https://review.openstack.org/620115
16:17:36 openstackgerrit sean mooney proposed openstack/nova master: libvirt: auto detach/attach sriov ports on migration https://review.openstack.org/629589
16:18:02 sean-k-mooney i fixed the merge conflict ^ otherwise its the same
16:23:59 openstackgerrit Rodolfo Alonso Hernandez proposed openstack/os-vif master: Retrieve the table schema using ovsdb-client command https://review.openstack.org/634967
16:36:43 artom sean-k-mooney, so, explain again why the nodeset in memoryBacking isn't necessary? If the dest compute has differently-numbered NUMA nodes, doesn't that need updating?
16:37:48 sean-k-mooney it is but its is carried in the numa_mappings info already
16:37:54 sean-k-mooney https://etherpad.openstack.org/p/numa_migration
16:40:34 sean-k-mooney i think the object i have listed in https://etherpad.openstack.org/p/numa_migration should have all the info you need?
16:42:10 artom sean-k-mooney, sorry, still don't get how if the source has <page size="1" unit="G" nodeset="4"/> and on the dest there is no nodeset 5 because there's only 4 NUMA nodes, how the source is supposed to update that bit of the XML

Earlier   Later