Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-06-05
19:27:03 mriedem most things in the virt driver layer are libvirt-biased
19:28:24 efried mriedem Don't I know it.
19:31:13 efried mriedem The other thing I noticed is that the only two consumes of get_info (in manager.py) only ever use the state. Makes me less panicky about hurrying to populate those other fields.
19:31:29 efried consumers*
19:47:27 efried jaypipes I presume the resource tracking stuff never plans to use get_info to figure out how much e.g. cpu/mem a VM is consuming - you're just going to record that at claim time and go with it?
19:50:05 dansmith efried: we don't know how much memory a guest is consuming without introspection
19:50:37 dansmith efried: and the amount of cpu it's using at some instantaneous point is not really an allocation thing
19:51:03 efried dansmith Okay, fair; but are there any plans for nova to actually use those InstanceInfo fields?
19:51:20 dansmith those...instanceinfo...fields...
19:51:23 efried Perhaps I'm missing some duck-typed dynamic usage of get_info
19:51:58 dansmith there are remnants of the extensible resource tracker and metric info in nova.. perhaps you mean that stuff?
19:52:18 efried dansmith https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L213-L217 <== those InstanceInfo fields
19:52:44 efried dansmith Perhaps I do mean that stuff. What stuff do I perhaps mean?
19:53:43 dansmith yeah, I _think_ that stuff is only used for choosing numa nodes
19:54:43 dansmith or maybe not even that, I don't even see that stuff considered in the scheduler
19:54:52 efried dansmith Does that happen in the nova tree? I'm even grepping and not finding any consumption of those fields.
19:55:55 dansmith efried: I think it's all within that file, and all for things like picking cpu topologies and stuff
19:56:05 dansmith but not a generic thing for reporting or choosing whole compute nodes, AFAIK
19:56:14 dansmith definitely not the material of placement though
19:56:45 efried dansmith What I'm considering doing here is leaving my InstanceInfo so it just provides the state. If anything ever gets written to consume any of the other fields, and assuming those code paths get hit in some tempest test, our CI will catch it (by raising AttributeError).
19:57:29 efried And at that time, we can figure out how best to implement those fields. Cause there's a number of options, with different performance characteristics, one of which would be more appropriate than another depending on the actual usage.
19:58:23 dansmith sounds reasonable
19:58:31 mriedem https://review.openstack.org/#/c/133777/
19:58:42 mriedem looks like it was added for something...but very little context
19:58:52 dansmith yeah
19:59:28 cfriesen_ weird python question....how is the "return" value from main() in cmd/manage.py expected to be consumed?
19:59:48 cfriesen_ it doesn't seem to have any effect on the process return value if run from the shell
19:59:49 efried Recently, too - but only relative to the aforementioned "by name (not ID!)" thing ;-)
20:02:14 mriedem goes back to https://github.com/openstack/nova/commit/19da125805eedbfcfd202abac4a90c57e6c538c4
20:02:49 mriedem https://github.com/openstack/nova/blob/c8e8b44ef27e49b3986659ee0cb6bd77b38430d8/nova/virt/libvirt_conn.py#L797
20:02:59 mriedem so, it just always returned that other crap
20:03:05 mriedem because we could
20:03:15 mriedem because the libvirt domain api gave it to us
20:04:00 mriedem https://github.com/openstack/nova/blob/c8e8b44ef27e49b3986659ee0cb6bd77b38430d8/nova/virt/xenapi/vm_utils.py#L686
20:05:51 efried Dig. I'll go with the plan, then. I'll just add a comment explaining it, so my posterity doesn't do a tim-the-tool-man like I did.
20:06:17 mriedem efried: there is one place where something besides 'state' is used, and it's in the libvirt driver's _destroy() method
20:06:23 mriedem checking to see if the domain id changed
20:07:23 mriedem unless there is another way to skin that id cat, we'd have to leave it,
20:07:33 mriedem but otherwise, if it were me, i'd remove all of that other unused garbage
20:07:36 mriedem from InstanceInfo
20:08:58 efried mriedem Okay, I'll look into it.
20:12:03 efried esberglu Can you answer cfriesen_ above? You figured out that the return values somehow mapped to exit codes?
20:15:40 openstack Launchpad bug 1695965 in OpenStack Compute (nova) ""nova-manage db online_data_migrations" exit code is strange" [Undecided,New]
20:15:40 cfriesen_ efried: esberglu: it appears that the return code is being mapped to the process exit value. it must be doing something clever. In any case, I opened https://bugs.launchpad.net/nova/+bug/1695965 because the return code for "nova-manage db online_data_migrations" isn't very helpful.
20:16:37 efried cfriesen_ Cool, esberglu just made a change on a similar theme: https://review.openstack.org/#/c/467218/
20:16:56 openstack Launchpad bug 1692982 in OpenStack Compute (nova) "Need --strict option for nova-manage cell_v2 discover_hosts" [Wishlist,Fix released] - Assigned to Eric Berglund (esberglu)
20:16:56 efried cfriesen_ See bug https://bugs.launchpad.net/nova/+bug/1692982
20:18:09 cfriesen_ efried: right. In the case of online_data_migrations we really want to distinguish between three separate scenarios, which doesn't map nicely to binary exit values
20:18:22 efried cfriesen_ In your case, it's possible the answer might be, "code to the API, not the CLI" ;-)
20:18:31 efried But I wouldn't be the one to advocate that.
20:19:32 cfriesen_ efried: heh...one nasty case is that the newton version of this command returns this "2 rows matched query pcidevice_online_data_migration, 0 migrated" even when there is no work left to do.
20:19:44 cfriesen_ outputs that, rather
20:20:01 cfriesen_ because the query is too generic.
20:21:37 efried cfriesen_ FWIW, dansmith was the one in our corner on that --strict thing. He had very useful feedback when the idea came up.
20:24:37 openstack bug 1692982 in OpenStack Compute (nova) "Need --strict option for nova-manage cell_v2 discover_hosts" [Wishlist,Fix released] https://launchpad.net/bugs/1692982 - Assigned to Eric Berglund (esberglu)
20:24:37 cfriesen_ dansmith: when you get a moment could you look at bug 1692982 ?
20:25:07 openstack bug 1695965 in OpenStack Compute (nova) ""nova-manage db online_data_migrations" exit code is strange" [Undecided,New] https://launchpad.net/bugs/1695965
20:25:07 cfriesen_ dansmith: whoops, I meant bug 1695965
20:25:47 esberglu efried: cfriesen_: Sorry was off for a few there. I think I dropped for some of the convo. You still need anything from me?
20:26:16 cfriesen_ esberglu: just wondering if you knew offhand how the return value of "nova-manage" gets converted to process exit value
20:26:17 dansmith cfriesen_: we had migrations in the past that raised an exception when something wasn't new enough, but you needed to proceed when in order to get to the newer stuff, hence the zero return in that case
20:26:37 dansmith thats why it was like that
20:26:43 dansmith but I don't disagree with nailing it down as you describe
20:27:22 cfriesen_ dansmith: also, in Newton you can't even tell from the output logs whether or not you need to call it again, because of the PCI issue I showed above
20:27:44 cfriesen_ dansmith: the "count_all" is counting cases where there is no migration to be done
20:28:11 esberglu efried: Not sure. I just took if granted based on other commands
20:47:41 openstack Launchpad bug 1695991 in OpenStack Compute (nova) ""nova-manage db online_data_migrations" doesn't report matched/migrated properly " [Undecided,New]
20:47:41 cfriesen_ dansmith: related issue: https://bugs.launchpad.net/nova/+bug/1695991
20:54:04 mriedem dansmith: i think we've touched on this before, but since services are restricted to cell dbs, we don't have a global view of what a minimum compute version is in a deployment once we have multpile cells
20:54:06 mriedem thinking of https://github.com/openstack/nova/blob/78c69f61ab186c3d3eb0fe5cc5114cc71f0fbdad/nova/objects/service.py#L425
20:54:36 dansmith mriedem: I have a helper
20:54:40 mriedem if we have an instance we can find the cell it's in and get the minimum compute for that cell, or get the service version for the compute the instance is on
20:55:59 cfriesen_ how is Service._check_minimum_version() going to work with cells then? check just the current cell?
20:56:16 mriedem well we could certainly iterate cells,
20:56:22 mriedem find the min for $service in all cells,
20:56:27 mriedem and return the lowest
20:56:42 dansmith mriedem: I'm sure I already wrote that helper.. looking
20:57:06 mriedem i randomly think about this because we don't really test for it in our grenade tests
20:57:12 mriedem since grenade is so limited in scope to smoke tests
20:57:59 mriedem e.g. https://github.com/openstack/nova/blob/78c69f61ab186c3d3eb0fe5cc5114cc71f0fbdad/nova/api/openstack/compute/servers.py#L496-L499
20:58:06 mriedem ^ won't be accurate with multiple cells
20:58:24 mriedem i'll open a bug to track this
20:58:37 dansmith and unless we had multiple real cells we wouldn't hit it anyway
20:58:53 mriedem we had == in ci?
20:59:27 dansmith yeah
20:59:33 mriedem i thought we were going to have the/a multinode job have 2 cells with 1 compute each?
20:59:44 dansmith yeah but not one downlevel
20:59:49 dansmith not yet anyway
20:59:51 mriedem sure, but we could with grenade
20:59:52 mriedem eventually
20:59:55 dansmith eventually
21:00:07 mriedem but then we're stuck back with the grenade not running interesting tests issue
21:00:18 dansmith that helper doesn't appear to be in tree, but I know I wrote it.. might've been for a thing that we ended up not doing and so never committed it
21:00:44 dansmith don't we run smoke first and then full tempest post-upgrade?
21:00:49 mriedem nope
21:00:50 mriedem smoke both ways
21:00:55 sdague no, it's smoke on both sides
21:01:11 mriedem smoke'em if you got'em
21:01:54 dansmith okay
21:02:03 sdague honestly, for the multi node, multi cell version testing, we should probably purpose build something for that. Because it feels like we want to be pushing very specific edges on test cases

Earlier   Later