[ovs-dev] [PATCH] ovsdb-idl: Mark row "parsed" in ovsdb_idl_txn_write__
dceara at redhat.com
Fri Jul 19 08:12:19 UTC 2019
Hi Damijan, Ben,
Damijan, sorry, I didn't realize you had already reported and fixed
the problem in your pending pull request. I wouldn't have sent my
Regarding a single parsed field instead of parsed bits per column I
had the impression that it's ok if unparsing is done for all columns.
Right now, whenever we set a column we call ovsdb_idl_txn_write__()
which unconditionally executes "(column->unparse)(row)" even if there
was no old value set before parsing the new datum. As we zero out rows
when we allocate them I assumed that this unparsing is safe.
What do you guys think?
On Fri, Jul 19, 2019 at 4:50 AM Damijan Skvarc <damjan.skvarc at gmail.com> wrote:
> On Thu, 18 Jul 2019, 20:12 Ben Pfaff, <blp at ovn.org> wrote:
>> I guess I missed it. Will you please point it out to me, for example in
>> the list archive?
>> On Thu, Jul 18, 2019 at 06:58:34PM +0200, Damijan Skvarc wrote:
>> > Hmm, the problem of "parsed" flag is that it identifies "all" columns of
>> > certain row have been parsed, however there are CLI tools which modify only
>> > individual colums by calling ovsdb_idl_txn_write_() function. In this case
>> > and in case parsed flag would be set in ovsdb_idl_txn_write() function then
>> > unparsing procedure would be called also for columns which were not parsed.
>> > The problem could be overcome by having individual flag for each column.
>> > This has been addresed in pending pull request. Apparent mail has been sent
>> > to dev list, but obviosly has been somehow overlooked.
>> > br, damijan
>> > On Thu, 18 Jul 2019, 17:52 Ben Pfaff, <blp at ovn.org> wrote:
>> > > On Wed, Jul 17, 2019 at 09:05:04PM +0200, Dumitru Ceara wrote:
>> > > > Once a column is set in a row using ovsdb_idl_txn_write__ we also mark
>> > > > the row "parsed". Otherwise the memory allocated by
>> > > > sbrec_<table>_parse_<col>() will never be freed. After marking the row
>> > > > "parsed", the ovsdb_idl_txn_disassemble function will properly free the
>> > > > newly allocated memory for the column (ovsdb_idl_row_unparse).
>> > >
>> > > Wow, good catch. I bet that's been there forever.
>> > >
>> > > The OVSDB IDL code is too complicated. It's too hard to spot the
>> > > issues. I wish I saw a way to make it simpler.
>> > > _______________________________________________
>> > > dev mailing list
>> > > dev at openvswitch.org
>> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> > >
More information about the dev