virt-test icon indicating copy to clipboard operation
virt-test copied to clipboard

qemu-kvm multi-queue doesn't play nice with Virtnet

Open cevich opened this issue 11 years ago • 1 comments

On 06/09/2013 11:09 AM, Amos Kong wrote:

self.virtnet[0]['tapfds'] = ['1', '2', '3', '4'] fail

self.virtnet[0]['tapfds'] = '1:2:3:4' good

That's curious. It certainly shouldn't be a problem on from the database side:

def save_to_db(self, db_key=None):
...
    data = str(self)

(It's saving the virtnet instance as a string of a python list of dict-like objects (also converted to strings).

The problem shouldn't be on the loading side either, since all values were saved as strings, and are re-formed using an eval:

def db_entry(self, db_key=None):
...
        db_entry = self.db[db_key]
...
        eval_result = eval(db_entry, {}, {})

I'm guessing the issue is in the string-conversion itself, where values are type-restricted:

class PropCan(PropCanBase):
    ...
    def __str__(self):
        acceptable_types = (str, unicode, int, float, long)
        return str( dict([(key, value) for key, value in self.items()
                           if issubclass(type(value), acceptable_types)]) )

That was done because the objects needed to be both "flat" and uniformly comparable to one-another, I also wasn't sure if the db engine could store anything other than string-values, so they made sense at the time. The limited types (above) are those which can be directly and uniformly converted to string values w/o any ambiguity or internal-quoting problems (which would mess up eval() on load).

However, in hind-sight, the shelve database can directly store python objects, and 'flatness' is probably not really required. So, it's possible everything could still work if we removed the type-limiting string conversion and eval(). Just simply plop each VirtIface instance into the database more or less as-is and be done with it. While a more 'correct' fix, this probably comes with the most risk of breaking something I can't foresee at the moment.

Another possibility for supporting storage of a list types, is to test them in str() above, to make sure they only store ints, floats, or longs. That should solve your specific issue, and introduce minimal risk. I'm not sure how extensively that change would affect the unittests, but that's easy to find out.

A third option is we get rid of shelve + address_pool database. Neither is required by libvirt, and dbnet could (maybe?) instead be hooked in with @ldoktor 's qemu-kvm ORM (https://github.com/autotest/virt-test/pull/522?source=cc). Though I'm not sure how far along it is or what kind of persistance it supports.

cevich avatar Jun 12 '13 13:06 cevich

Yes, the return of str(self) in save_to_db() didn't contain dict items.

amoskong avatar Jun 13 '13 02:06 amoskong