Re: [libvirt] [PATCH 0/3] Transport Open vSwitch per-port data during live migration

On 10/22/2012 09:41 AM, Kyle Mestery (kmestery) wrote: > On Oct 11, 2012, at 4:36 PM, Kyle Mestery (kmestery) <kmestery(a)cisco.com&gt; wrote: >> On Oct 11, 2012, at 4:25 PM, Laine Stump <laine(a)laine.org&gt; wrote: >>> On 10/11/2012 05:06 PM, Kyle Mestery (kmestery) wrote: >>>> On Oct 1, 2012, at 10:18 AM, Kyle Mestery <kmestery(a)cisco.com&gt; wrote: >>>>> This series of commits has the end goal of allowing per-port data stored >>>>> in the Open vSwitch DB to be transported during live migration. This is >>>>> done by first providing a generic infrastructure for transporting network >>>>> data, adding some utility functions specific to Open vSwitch, and hooking >>>>> the two together. >>>>> >>>>> The framework provided is generic in that other networking data could be >>>>> transferred as well by simply adding in additional hooks as needed. >>>>> >>>>> Kyle Mestery (3): >>>>> Add the ability for the Qemu V3 migration protocol to include >>>>> transporting network configuration. A generic framework is proposed >>>>> with this patch to allow for the transfer of opaque data. >>>>> Add utility functions for Open vSwitch to both save per-port data >>>>> before a live migration, and restore the per-port data after a >>>>> live migration. >>>>> Transport Open vSwitch per-port data during live migration by >>>>> using the utility functions >>>>> virNetDevOpenvswitchGetMigrateData() and >>>>> virNetDevOpenvswitchSetMigrateData(). >>>>> >>>>> src/libvirt_private.syms | 2 + >>>>> src/qemu/qemu_migration.c | 263 +++++++++++++++++++++++++++++++++++++++- >>>>> src/util/virnetdevopenvswitch.c | 70 +++++++++++ >>>>> src/util/virnetdevopenvswitch.h | 6 + >>>>> 4 files changed, 339 insertions(+), 2 deletions(-) >>>> Just curious if anyone has had a chance to review this series yet? I believe I've addressed all the comments >>>> I've received so far. I may need to rebase and send it out again since it's been almost 2 weeks since I sent it >>>> out. >>> Sorry I haven't responded. Lately it's been one deadline after another >>> (actually I'm working on the latest as I type). >>> >> Thanks Laine! >> >>> One thought I've had about this - since you're just taking the data >>> directly from ovs-vsctl and printf-ing it into a buffer, there is a >>> potential that the data could contain xml meta characters (either by >>> design / on purpose, or perhaps if an attacker takes over ovs-vsctl or >>> the database in some way). This could leave the system that's the target >>> of the migration open to attacks based on injecting "something else" >>> into the migration cookie. >>> >>> Is virBufferEscapeString() enough to both guarantee nothing like that >>> happens, as well as getting the exact same string at the destination? I >>> *think* so, but am not sure. >>> >>> Also, I'm still not so sure about having the data as an attribute when >>> it could potentially been very long. DV, what's your opinion about this? >>> Is it okay to have very long strings as attributes, or is it considered >>> in better taste to put something that is, say, 1000 characters long as a >>> separate element? >>> >>> Anyway, at this point I don't think you need to rebase/resend. By the >>> beginning of next week hopefully someone more wise than me about XML >>> will have answered these two questions, and I can just squash in the >>> minor changes and push it. >> Thanks Laine! Let me know if you need anything else on my end. >> >> Thanks, >> Kyle > > Just pinging again on this patch set Laine. I don't think anyone responded on > your XML questions yet, so I'm unsure what the current status of this patch set > is. I assume it's in the same state? Yes. That was to be my first task this morning, but I arrived at the keyboard to a string of questions on IRC. I'll hopefully post something for you to test before noon (EST). If it works properly, and you approve of the changes, then I'll push.